* [PATCH for-net] net/mlx4_en: Check device state when setting coalescing
From: Amir Vadai @ 2013-09-12 15:11 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Amir Vadai, Or Gerlitz, Gideon Naim, Eugenia Emantayev
From: Eugenia Emantayev <eugenia@mellanox.com>
When the device is down, CQs are freed. We must check the device state
to avoid issuing firmware commands on non existing CQs.
CC: Or Gerlitz <ogerlitz@mellanox.com>
Signed-off-by: Eugenia Emantayev <eugenia@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/en_ethtool.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
index a28cd80..0c75098 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
@@ -53,9 +53,11 @@ static int mlx4_en_moderation_update(struct mlx4_en_priv *priv)
for (i = 0; i < priv->tx_ring_num; i++) {
priv->tx_cq[i].moder_cnt = priv->tx_frames;
priv->tx_cq[i].moder_time = priv->tx_usecs;
- err = mlx4_en_set_cq_moder(priv, &priv->tx_cq[i]);
- if (err)
- return err;
+ if (priv->port_up) {
+ err = mlx4_en_set_cq_moder(priv, &priv->tx_cq[i]);
+ if (err)
+ return err;
+ }
}
if (priv->adaptive_rx_coal)
@@ -65,9 +67,11 @@ static int mlx4_en_moderation_update(struct mlx4_en_priv *priv)
priv->rx_cq[i].moder_cnt = priv->rx_frames;
priv->rx_cq[i].moder_time = priv->rx_usecs;
priv->last_moder_time[i] = MLX4_EN_AUTO_CONF;
- err = mlx4_en_set_cq_moder(priv, &priv->rx_cq[i]);
- if (err)
- return err;
+ if (priv->port_up) {
+ err = mlx4_en_set_cq_moder(priv, &priv->rx_cq[i]);
+ if (err)
+ return err;
+ }
}
return err;
--
1.8.3.4
^ permalink raw reply related
* Re: [PATCH v2] Don't destroy the netdev until the vif is shut down
From: Wei Liu @ 2013-09-12 15:21 UTC (permalink / raw)
To: Paul Durrant; +Cc: netdev, xen-devel, David Vrabel, Wei Liu, Ian Campbell
In-Reply-To: <1378988092-15443-1-git-send-email-paul.durrant@citrix.com>
The title should start with "[PATCH net-next]".
On Thu, Sep 12, 2013 at 01:14:52PM +0100, Paul Durrant wrote:
> Without this patch, if a frontend cycles through states Closing
> and Closed (which Windows frontends need to do) then the netdev
> will be destroyed and requires re-invocation of hotplug scripts
> to restore state before the frontend can move to Connected. Thus
> when udev is not in use the backend gets stuck in InitWait.
>
> With this patch, the netdev is left alone whilst the backend is
> still online and is only de-registered and freed just prior to
> destroying the vif (which is also nicely symmetrical with the
> netdev allocation and registration being done during probe) so
> no re-invocation of hotplug scripts is required.
>
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Cc: David Vrabel <david.vrabel@citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> ---
> v2:
> - Modify netback_remove() - bug only seemed to manifest with linux guest
>
> drivers/net/xen-netback/common.h | 1 +
> drivers/net/xen-netback/interface.c | 13 ++++++++-----
> drivers/net/xen-netback/xenbus.c | 17 ++++++++++++-----
> 3 files changed, 21 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
> index a197743..5715318 100644
> --- a/drivers/net/xen-netback/common.h
> +++ b/drivers/net/xen-netback/common.h
> @@ -184,6 +184,7 @@ int xenvif_connect(struct xenvif *vif, unsigned long tx_ring_ref,
> unsigned long rx_ring_ref, unsigned int tx_evtchn,
> unsigned int rx_evtchn);
> void xenvif_disconnect(struct xenvif *vif);
> +void xenvif_free(struct xenvif *vif);
>
This can be moved a few lines above to group with xenvif_alloc() IMHO.
> int xenvif_xenbus_init(void);
> void xenvif_xenbus_fini(void);
> diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
> index 625c6f4..65e78f9 100644
> --- a/drivers/net/xen-netback/interface.c
> +++ b/drivers/net/xen-netback/interface.c
> @@ -477,14 +477,17 @@ void xenvif_disconnect(struct xenvif *vif)
> if (vif->task)
> kthread_stop(vif->task);
>
> + xenvif_unmap_frontend_rings(vif);
> +
> + if (need_module_put)
> + module_put(THIS_MODULE);
> +}
> +
The original thought for this module_get/put thing is that admin can
just disconnect all frontends then replace netback module. With this
patch that doesn't work anymore. We leak memory when all frontends are
disconnected and admin runs "modprobe -r xen-netback". That happened to
work before ecause everytime a frontend is disconnected the netdev structure
is already freed.
The ability to unload netback is still quite handy for developer / admin
so I would like to keep it. My suggestion would be, move module_get/put
to the point where netdev is allocated / freed. The impact of moving
module_get/put is that we cannot just simply disconnect all frontends
then replace netback, we need to actually shutdown / migrate all VMs
before replacing netback.
Wei.
^ permalink raw reply
* Re: [PATCH net-next] fix NULL pointer dereference in br_handle_frame
From: Eric Dumazet @ 2013-09-12 15:24 UTC (permalink / raw)
To: vyasevic; +Cc: Hong zhi guo, netdev, David Miller, zhiguohong
In-Reply-To: <5231D2D2.3090907@redhat.com>
On Thu, 2013-09-12 at 10:42 -0400, Vlad Yasevich wrote:
> Don't all tests for IFF_BRIDGE_PORT on the bridge receive path become
> redundant as well?
Sure but anyway this part is net-next material
Lets fix the bug first.
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* RE: [PATCH v2] Don't destroy the netdev until the vif is shut down
From: Paul Durrant @ 2013-09-12 15:30 UTC (permalink / raw)
To: Wei Liu
Cc: netdev@vger.kernel.org, xen-devel@lists.xen.org, David Vrabel,
Wei Liu, Ian Campbell
In-Reply-To: <20130912152122.GC934@zion.uk.xensource.com>
> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@citrix.com]
> Sent: 12 September 2013 16:21
> To: Paul Durrant
> Cc: netdev@vger.kernel.org; xen-devel@lists.xen.org; David Vrabel; Wei Liu;
> Ian Campbell
> Subject: Re: [PATCH v2] Don't destroy the netdev until the vif is shut down
>
> The title should start with "[PATCH net-next]".
>
Ok.
> On Thu, Sep 12, 2013 at 01:14:52PM +0100, Paul Durrant wrote:
> > Without this patch, if a frontend cycles through states Closing
> > and Closed (which Windows frontends need to do) then the netdev
> > will be destroyed and requires re-invocation of hotplug scripts
> > to restore state before the frontend can move to Connected. Thus
> > when udev is not in use the backend gets stuck in InitWait.
> >
> > With this patch, the netdev is left alone whilst the backend is
> > still online and is only de-registered and freed just prior to
> > destroying the vif (which is also nicely symmetrical with the
> > netdev allocation and registration being done during probe) so
> > no re-invocation of hotplug scripts is required.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > Cc: David Vrabel <david.vrabel@citrix.com>
> > Cc: Wei Liu <wei.liu2@citrix.com>
> > Cc: Ian Campbell <ian.campbell@citrix.com>
> > ---
> > v2:
> > - Modify netback_remove() - bug only seemed to manifest with linux
> guest
> >
> > drivers/net/xen-netback/common.h | 1 +
> > drivers/net/xen-netback/interface.c | 13 ++++++++-----
> > drivers/net/xen-netback/xenbus.c | 17 ++++++++++++-----
> > 3 files changed, 21 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-
> netback/common.h
> > index a197743..5715318 100644
> > --- a/drivers/net/xen-netback/common.h
> > +++ b/drivers/net/xen-netback/common.h
> > @@ -184,6 +184,7 @@ int xenvif_connect(struct xenvif *vif, unsigned long
> tx_ring_ref,
> > unsigned long rx_ring_ref, unsigned int tx_evtchn,
> > unsigned int rx_evtchn);
> > void xenvif_disconnect(struct xenvif *vif);
> > +void xenvif_free(struct xenvif *vif);
> >
>
> This can be moved a few lines above to group with xenvif_alloc() IMHO.
>
A matter of personal taste I think.
> > int xenvif_xenbus_init(void);
> > void xenvif_xenbus_fini(void);
> > diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-
> netback/interface.c
> > index 625c6f4..65e78f9 100644
> > --- a/drivers/net/xen-netback/interface.c
> > +++ b/drivers/net/xen-netback/interface.c
> > @@ -477,14 +477,17 @@ void xenvif_disconnect(struct xenvif *vif)
> > if (vif->task)
> > kthread_stop(vif->task);
> >
> > + xenvif_unmap_frontend_rings(vif);
> > +
> > + if (need_module_put)
> > + module_put(THIS_MODULE);
> > +}
> > +
>
> The original thought for this module_get/put thing is that admin can
> just disconnect all frontends then replace netback module. With this
> patch that doesn't work anymore. We leak memory when all frontends are
> disconnected and admin runs "modprobe -r xen-netback". That happened
> to
> work before ecause everytime a frontend is disconnected the netdev
> structure
> is already freed.
>
> The ability to unload netback is still quite handy for developer / admin
> so I would like to keep it. My suggestion would be, move module_get/put
> to the point where netdev is allocated / freed. The impact of moving
> module_get/put is that we cannot just simply disconnect all frontends
> then replace netback, we need to actually shutdown / migrate all VMs
> before replacing netback.
>
Presumably you still had the problem of needing to manually re-run the hotplug scripts if not using udev though. I'll move the module_get/put as you suggest.
Paul
^ permalink raw reply
* [PATCH 3/3] wlcore: limit base for output buffer in dev_mem_read
From: Vladimir Murzin @ 2013-09-12 15:32 UTC (permalink / raw)
To: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
Cc: coelho-l0cyMroinI0, linville-2XuSBdqkA4R54TAoqtyWWQ,
Vladimir Murzin
In-Reply-To: <1378999943-1968-1-git-send-email-murzin.v-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
dev_mem_read tries to speed up things a bit by limiting output
buffer which can not exceed WLCORE_MAX_BLOCK_SIZE. However,
WLCORE_MAX_BLOCK_SIZE is based on PAGE_SIZE which may vary.
Use 4K size base for buffer explicitly.
Signed-off-by: Vladimir Murzin <murzin.v-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
drivers/net/wireless/ti/wlcore/debugfs.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ti/wlcore/debugfs.c b/drivers/net/wireless/ti/wlcore/debugfs.c
index e17630c..6b413a7 100644
--- a/drivers/net/wireless/ti/wlcore/debugfs.c
+++ b/drivers/net/wireless/ti/wlcore/debugfs.c
@@ -38,7 +38,8 @@
/* ms */
#define WL1271_DEBUGFS_STATS_LIFETIME 1000
-#define WLCORE_MAX_BLOCK_SIZE ((size_t)(4*PAGE_SIZE))
+#define WLCORE_PAGE_SIZE 4096
+#define WLCORE_MAX_BLOCK_SIZE ((size_t)(4*WLCORE_PAGE_SIZE))
/* debugfs macros idea from mac80211 */
int wl1271_format_buffer(char __user *userbuf, size_t count,
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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
* small PAGE_SIZE related fixes for wl{12,18}xx drivers
From: Vladimir Murzin @ 2013-09-12 15:32 UTC (permalink / raw)
To: linux-wireless, netdev; +Cc: coelho, linville
This small series is considering PAGE_SIZE usage into the wl drivers. I have
no real HW to test it tightly, probably these drivers will never run on arches
with PAGE_SIZE different to 4K. Nevertheless, I hope this mini-series (or some
part of it) will be useful for you.
Thanks.
Vladimir
^ permalink raw reply
* [PATCH 1/3] wlcore: fix frame size overflow warning in wl12xx_spi_raw_write
From: Vladimir Murzin @ 2013-09-12 15:32 UTC (permalink / raw)
To: linux-wireless, netdev; +Cc: coelho, linville, Vladimir Murzin
In-Reply-To: <1378999943-1968-1-git-send-email-murzin.v@gmail.com>
While cross-building for PPC64 I've got
drivers/net/wireless/ti/wlcore/spi.c: In function
'wl12xx_spi_raw_write': drivers/net/wireless/ti/wlcore/spi.c:317:1:
warning: the frame size of 9712 bytes is larger than 2048 bytes
[-Wframe-larger-than=]
WSPI_MAX_NUM_OF_CHUNKS depends on SPI_AGGR_BUFFER_SIZE which in turn
is based on PAGE_SIZE. For most systems PAGE_SIZE is stands for 4K,
but it may vary - in my case PAGE_SIZE is 64K.
Fix calculation by using 4K explicitly.
Signed-off-by: Vladimir Murzin <murzin.v@gmail.com>
---
drivers/net/wireless/ti/wlcore/spi.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ti/wlcore/spi.c b/drivers/net/wireless/ti/wlcore/spi.c
index 1b0cd98..8dce028 100644
--- a/drivers/net/wireless/ti/wlcore/spi.c
+++ b/drivers/net/wireless/ti/wlcore/spi.c
@@ -70,7 +70,8 @@
* only support SPI for 12xx - this code should be reworked when 18xx
* support is introduced
*/
-#define SPI_AGGR_BUFFER_SIZE (4 * PAGE_SIZE)
+#define SPI_PAGE_SIZE 4096
+#define SPI_AGGR_BUFFER_SIZE (4 * SPI_PAGE_SIZE)
#define WSPI_MAX_NUM_OF_CHUNKS (SPI_AGGR_BUFFER_SIZE / WSPI_MAX_CHUNK_SIZE)
--
1.7.10.4
^ permalink raw reply related
* [PATCH 2/3] wl12xx/wl18xx: limit base for aggregate buffer size to 4K
From: Vladimir Murzin @ 2013-09-12 15:32 UTC (permalink / raw)
To: linux-wireless, netdev; +Cc: coelho, linville, Vladimir Murzin
In-Reply-To: <1378999943-1968-1-git-send-email-murzin.v@gmail.com>
WL{12,18}XX_AGGR_BUFFER_SIZE is depends on PAGE_SIZE which may be more
than 4K. In this case memory might be aggressively wasted.
Use 4K size base for buffer explicitly.
Signed-off-by: Vladimir Murzin <murzin.v@gmail.com>
---
drivers/net/wireless/ti/wl12xx/wl12xx.h | 3 ++-
drivers/net/wireless/ti/wl18xx/wl18xx.h | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ti/wl12xx/wl12xx.h b/drivers/net/wireless/ti/wl12xx/wl12xx.h
index 9e5484a..3649d40 100644
--- a/drivers/net/wireless/ti/wl12xx/wl12xx.h
+++ b/drivers/net/wireless/ti/wl12xx/wl12xx.h
@@ -56,7 +56,8 @@
#define WL128X_SUBTYPE_MR_VER WLCORE_FW_VER_IGNORE
#define WL128X_MINOR_MR_VER 42
-#define WL12XX_AGGR_BUFFER_SIZE (4 * PAGE_SIZE)
+#define WL12XX_PAGE_SIZE 4096
+#define WL12XX_AGGR_BUFFER_SIZE (4 * WL12XX_PAGE_SIZE)
#define WL12XX_NUM_TX_DESCRIPTORS 16
#define WL12XX_NUM_RX_DESCRIPTORS 8
diff --git a/drivers/net/wireless/ti/wl18xx/wl18xx.h b/drivers/net/wireless/ti/wl18xx/wl18xx.h
index 9204e07..a3214b4 100644
--- a/drivers/net/wireless/ti/wl18xx/wl18xx.h
+++ b/drivers/net/wireless/ti/wl18xx/wl18xx.h
@@ -33,7 +33,8 @@
#define WL18XX_CMD_MAX_SIZE 740
-#define WL18XX_AGGR_BUFFER_SIZE (13 * PAGE_SIZE)
+#define WL18XX_PAGE_SIZE 4096
+#define WL18XX_AGGR_BUFFER_SIZE (13 * WL18XX_PAGE_SIZE)
#define WL18XX_NUM_TX_DESCRIPTORS 32
#define WL18XX_NUM_RX_DESCRIPTORS 32
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH net-next] fix NULL pointer dereference in br_handle_frame
From: Vlad Yasevich @ 2013-09-12 15:50 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Hong zhi guo, netdev, David Miller, zhiguohong
In-Reply-To: <1378999470.24408.10.camel@edumazet-glaptop>
On 09/12/2013 11:24 AM, Eric Dumazet wrote:
> On Thu, 2013-09-12 at 10:42 -0400, Vlad Yasevich wrote:
>
>> Don't all tests for IFF_BRIDGE_PORT on the bridge receive path become
>> redundant as well?
>
> Sure but anyway this part is net-next material
>
> Lets fix the bug first.
Seems like the race is possible in net as well. If we just include the
original patch, then br_add_if() also has to be modified to make sure
the flag is set properly before netdev_rx_handler_register() is called.
If we remove the need to check the flag during bridge input handling,
then br_add_if() change is not necessary.
-vlad
>
> Acked-by: Eric Dumazet <edumazet@google.com>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [ethtool] ethtool: correct ixgbevf copyright date
From: Ben Hutchings @ 2013-09-12 14:17 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: Jacob Keller, netdev, gospo, sassmann
In-Reply-To: <1377925372-13385-1-git-send-email-jeffrey.t.kirsher@intel.com>
On Fri, 2013-08-30 at 22:02 -0700, Jeff Kirsher wrote:
> From: Jacob Keller <jacob.e.keller@intel.com>
>
> This patch corrects the copyright date of the ixgbevf code, which was only
> recently added.
Applied, thanks.
Ben.
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
> ixgbevf.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ixgbevf.c b/ixgbevf.c
> index dae1178..2a3aa6f 100644
> --- a/ixgbevf.c
> +++ b/ixgbevf.c
> @@ -1,4 +1,4 @@
> -/* Copyright (c) 2007 Intel Corporation */
> +/* Copyright (c) 2013 Intel Corporation */
> #include <stdio.h>
> #include "internal.h"
>
--
Ben Hutchings, Staff 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 20/52] net: fealnx: remove unnecessary pci_set_drvdata()
From: Ben Hutchings @ 2013-09-12 13:16 UTC (permalink / raw)
To: David Miller; +Cc: jg1.han, sergei.shtylyov, netdev
In-Reply-To: <20130912.033606.1689936899104585741.davem@davemloft.net>
On Thu, 2013-09-12 at 03:36 -0400, David Miller wrote:
> From: Jingoo Han <jg1.han@samsung.com>
> Date: Thu, 12 Sep 2013 14:49:56 +0900
>
> > On Thursday, September 12, 2013 2:25 PM, David Miller wrote:
> >> On Thu, 12 Sep 2013 09:11:01 +0900, Jingoo Han wrote:
> >> > Would you let know the reason not to add coding style fixes?
> >>
> >> They should be made seperately so that the individual changes
> >> can be reviewed more easily and without unnecessary unrelated
> >> changes mixed in.
> >
> > OK, I see. :-)
> > Thank you for your answer.
> > Then, I will send V2 patch soon.
>
> Please do not submit such a huge patch series in the future.
>
> Anything more than 16 patches at a time is not reasonable and
> overloads reviewer's capacity to look at your changes.
Given that this is the same trivial change applied to many different
drivers, is it really worthwhile to break it up into a patch per driver?
Ben.
--
Ben Hutchings, Staff 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: [ethtool 1/3] ethtool: fix ixgbe 82598EB only registers
From: Ben Hutchings @ 2013-09-12 15:45 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: Jacob Keller, netdev, gospo, sassmann
In-Reply-To: <1377809470.5372.16.camel@bwh-desktop.uk.level5networks.com>
On Thu, 2013-08-29 at 21:51 +0100, Ben Hutchings wrote:
> On Tue, 2013-08-27 at 17:08 -0700, Jeff Kirsher wrote:
> > From: Jacob Keller <jacob.e.keller@intel.com>
> >
> > This patch fixes ixgbe_reg_dump to only display registers specific to the
> > 82598EB part, as these registers display 0xDEADBEEF otherwise, and clutter up
> > the register dump.
> >
> > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> > Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > ---
> > ixgbe.c | 144 +++++++++++++++++++++++++++++++++-------------------------------
> > 1 file changed, 75 insertions(+), 69 deletions(-)
> >
> > diff --git a/ixgbe.c b/ixgbe.c
> > index 9b005f2..89cb6be 100644
> > --- a/ixgbe.c
> > +++ b/ixgbe.c
> [...]
> > - fprintf(stdout,
> > - "0x02F20: RDPROBE (Rx Probe Mode Status) 0x%08X\n",
> > - regs_buff[1085]);
> [...]
>
> It looks like this removal really belongs to the next patch, which adds
> it back with the mac_type < ixgbe_mac_X540 condition.
Well, I've applied the two patches with this bit fixed up.
Ben.
--
Ben Hutchings, Staff 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: ip l2tp - suspected defect using IPv6 local/remote addresses
From: Ben Hutchings @ 2013-09-12 13:19 UTC (permalink / raw)
To: James Chapman; +Cc: Jeff Loughridge, netdev
In-Reply-To: <CAEwTi7Qo8=X=v51_1y3JMQnbG3xBQYRb9YRePFh6KfYmb_Y0-g@mail.gmail.com>
On Thu, 2013-09-12 at 11:20 +0100, James Chapman wrote:
> On 11 September 2013 19:52, Jeff Loughridge <jeffl@alumni.duke.edu> wrote:
> >
> > Using IPv6 address as L2TPv3 endpoints doesn't seem to work in
> > iproute2 3.11. I see a cosmetic defect in the output of 'ip l2tp show
> > tunnel'. In addition, I can't get tunnels to function with UDP or IP
> > encapsulation.
> >
> > root@debian:~# ip l2tp add tunnel tunnel_id 3000 peer_tunnel_id 4000
> > encap udp local a::1 remote a::2 udp_sport 5000 udp_dport 6000
> > root@debian:~# ip l2tp add session tunnel_id 3000 session_id 1000
> > peer_session_id 2000
> > root@debian:~# ip l2tp show tunnel
> > Tunnel 3000, encap UDP
> > From 127.0.0.1 to 127.0.0.1
> > Peer tunnel 4000
> > UDP source / dest ports: 5000/6000
> > root@debian:~#
>
> You'll need a 3.5 or later kernel for L2TP over IPv6. I see you are
> using 3.2. Are you using a version of iproute2 which is not matched to
> your kernel?
[...]
The iproute2 version number only indicates which kernel features it
supports; it is supposed to be backward-compatible. And it really
should not silently fail like this, although this may well be a bug in
the API that can't be fixed in userland...
Ben.
--
Ben Hutchings, Staff 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
* [PATCH net-next] fix NULL pointer dereference in br_handle_frame
From: Hong Zhiguo @ 2013-09-12 15:57 UTC (permalink / raw)
To: vyasevic; +Cc: eric.dumazet, davem, netdev, Hong Zhiguo
In-Reply-To: <1378988195-2710-1-git-send-email-zhiguohong@tencent.com>
not check IFF_BRIDGE_PORT within br_handle_frame
Signed-off-by: Hong Zhiguo <zhiguohong@tencent.com>
---
net/bridge/br_input.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index a2fd37e..da4714a 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -60,7 +60,7 @@ static int br_pass_frame_up(struct sk_buff *skb)
int br_handle_frame_finish(struct sk_buff *skb)
{
const unsigned char *dest = eth_hdr(skb)->h_dest;
- struct net_bridge_port *p = br_port_get_rcu(skb->dev);
+ enet_bridge_port *p = rcu_dereference(skb->dev->rx_handler_data);
struct net_bridge *br;
struct net_bridge_fdb_entry *dst;
struct net_bridge_mdb_entry *mdst;
@@ -143,7 +143,7 @@ drop:
/* note: already called with rcu_read_lock */
static int br_handle_local_finish(struct sk_buff *skb)
{
- struct net_bridge_port *p = br_port_get_rcu(skb->dev);
+ struct net_bridge_port *p = rcu_dereference(skb->dev->rx_handler_data);
u16 vid = 0;
br_vlan_get_tag(skb, &vid);
@@ -173,7 +173,7 @@ rx_handler_result_t br_handle_frame(struct sk_buff **pskb)
if (!skb)
return RX_HANDLER_CONSUMED;
- p = br_port_get_rcu(skb->dev);
+ p = rcu_dereference(skb->dev->rx_handler_data);
if (unlikely(is_link_local_ether_addr(dest))) {
/*
--
1.7.0.4
^ permalink raw reply related
* ethtool 3.11 released
From: Ben Hutchings @ 2013-09-12 15:58 UTC (permalink / raw)
To: netdev
ethtool version 3.11 has been released.
Home page: https://ftp.kernel.org/pub/software/network/ethtool/
Download link:
https://ftp.kernel.org/pub/software/network/ethtool/ethtool-3.11.tar.xz
Release notes:
* Feature: Update Realtek chip list for register dump to match
r8169 driver in Linux 3.11 (-d option)
* Feature: Add ixgbevf support for register dump (-d option)
* Feature: Filter ixgbe register dump according to the specific chip
(-d option)
Ben.
--
Ben Hutchings, Staff 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: [RFC PATCH] vsnprintf: Remove use of %n and convert existing uses
From: Joe Perches @ 2013-09-12 15:59 UTC (permalink / raw)
To: David Laight
Cc: Al Viro, Tetsuo Handa, linux-kernel, kosaki.motohiro, keescook,
fweisbec, dan.carpenter, devel, gregkh, tushar.behera,
lidza.louina, davem, kuznet, jmorris, yoshfuji, kaber, courmisch,
vyasevich, nhorman, netdev, linux-sctp
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B732A@saturn3.aculab.com>
On Thu, 2013-09-12 at 09:06 +0100, David Laight wrote:
> > On Wed, Sep 11, 2013 at 05:04:17PM -0700, Joe Perches wrote:
> > > On Thu, 2013-09-12 at 08:40 +0900, Tetsuo Handa wrote:
> > > > Joe Perches wrote:
> > > > > - seq_printf(m, "%s%d%n", con->name, con->index, &len);
> > > > > + len = seq_printf(m, "%s%d", con->name, con->index);
> > > >
> > > > Isn't len always 0 or -1 ?
> > >
> > > Right. Well you're no fun...
[]
> > > Suggestions?
>
> Change the return type of seq_printf() to void and require that
> code use access functions/macros to find the length and error
> status. Possibly save the length of the last call separately.
Are there any races if this is done?
---
fs/seq_file.c | 15 +++++++--------
include/linux/seq_file.h | 6 ++++--
2 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/fs/seq_file.c b/fs/seq_file.c
index 3135c25..b6c9eab 100644
--- a/fs/seq_file.c
+++ b/fs/seq_file.c
@@ -389,32 +389,31 @@ int seq_escape(struct seq_file *m, const char *s, const char *esc)
}
EXPORT_SYMBOL(seq_escape);
-int seq_vprintf(struct seq_file *m, const char *f, va_list args)
+void seq_vprintf(struct seq_file *m, const char *f, va_list args)
{
int len;
if (m->count < m->size) {
len = vsnprintf(m->buf + m->count, m->size - m->count, f, args);
+ m->last_len = len;
if (m->count + len < m->size) {
m->count += len;
- return 0;
+ m->last_rtn = 0;
+ return;
}
}
seq_set_overflow(m);
- return -1;
+ m->last_rtn = -1;
}
EXPORT_SYMBOL(seq_vprintf);
-int seq_printf(struct seq_file *m, const char *f, ...)
+void seq_printf(struct seq_file *m, const char *f, ...)
{
- int ret;
va_list args;
va_start(args, f);
- ret = seq_vprintf(m, f, args);
+ seq_vprintf(m, f, args);
va_end(args);
-
- return ret;
}
EXPORT_SYMBOL(seq_printf);
diff --git a/include/linux/seq_file.h b/include/linux/seq_file.h
index 4e32edc..9af05e1 100644
--- a/include/linux/seq_file.h
+++ b/include/linux/seq_file.h
@@ -20,6 +20,8 @@ struct seq_file {
size_t size;
size_t from;
size_t count;
+ size_t last_len;
+ int last_rtn;
loff_t index;
loff_t read_pos;
u64 version;
@@ -89,8 +91,8 @@ int seq_putc(struct seq_file *m, char c);
int seq_puts(struct seq_file *m, const char *s);
int seq_write(struct seq_file *seq, const void *data, size_t len);
-__printf(2, 3) int seq_printf(struct seq_file *, const char *, ...);
-__printf(2, 0) int seq_vprintf(struct seq_file *, const char *, va_list args);
+__printf(2, 3) void seq_printf(struct seq_file *, const char *, ...);
+__printf(2, 0) void seq_vprintf(struct seq_file *, const char *, va_list args);
int seq_path(struct seq_file *, const struct path *, const char *);
int seq_dentry(struct seq_file *, struct dentry *, const char *);
^ permalink raw reply related
* Re: [PATCH 14/52] net: cxgb4vf: remove unnecessary pci_set_drvdata()
From: Casey Leedom @ 2013-09-12 15:59 UTC (permalink / raw)
To: Jingoo Han; +Cc: 'David S. Miller', netdev
In-Reply-To: <000001ceaf56$1cdf7d60$569e7820$%han@samsung.com>
Ah, okay. And I presume the same is true of the PCI Remove path. I
wasn't aware of this but if it's the Linux pattern to let the
surrounding support code do this work, then go for it.
As I said, I'm something of a Mad Housewife when it comes to cleaning
things up which were allocated/assigned in a particular routine.
Basically I program like I drive, practising defensive programming ... :-)
Casey
On 09/11/13 18:19, Jingoo Han wrote:
> On Thursday, September 12, 2013 2:24 AM, Casey Leedom wrote:
>> I agree that the redundant pci_set_drvdata(pdev, NULL) in
>> cxgb4vf_pci_probe() under the err_release_regions: label is unneeded,
>> but don't we need to NULL out the PCI Driver Data under the
>> err_free_adapter: label and also in cxgb4vf_pci_remove()? Or is that
>> handled automatically in the PCI infrastructure code which calls the
>> Device Probe and Remove routines? Mostly I was just being an
>> obsessively clean housewife assuming that we'd want to clean up these
>> references ...
>
> No, 'pci_set_drvdata(pdev, NULL) under err_free_adapter label' is not
> necessary.
>
> As you know, pci_set_drvdata(pdev, NULL) calls dev_set_drvdata() as below:
> pci_set_drvdata(pdev, NULL) is dev_set_drvdata(&pdev->dev, NULL).
>
> ./include/linux/pci.h
> 1504:static inline void pci_set_drvdata(struct pci_dev *pdev, void *data)
> 1505{
> 1506 dev_set_drvdata(&pdev->dev, data);
> 1507}
>
>
> However, when the driver goes to err_free_adapter label,
> The following sequence will be done.
> kfree(adapter) -> .... -> return -ENOMEM;
>
> In this case,
> when probe() returns error value such as '-ENOMEM',
> really_probe() of driver core automatically calls 'dev_set_drvdata(dev, NULL)'
> as below:
>
> ./drivers/base/dd.c
> 303-probe_failed:
> 304 devres_release_all(dev);
> 305 driver_sysfs_remove(dev);
> 306 dev->driver = NULL;
> 307 dev_set_drvdata(dev, NULL);
>
> Thus, without 'pci_set_drvdata(pdev, NULL) under err_free_adapter label',
> dev_set_drvdata(dev, NULL) can be called.
>
> I already tested this with other drivers such as e1000e LAN card driver.
>
> Best regards,
> Jingoo Han
>
>
^ permalink raw reply
* Re: [PATCH net-next] fix NULL pointer dereference in br_handle_frame
From: Hong zhi guo @ 2013-09-12 16:06 UTC (permalink / raw)
To: vyasevic; +Cc: Eric Dumazet, David Miller, netdev, Hong Zhiguo
In-Reply-To: <1379001428-2727-1-git-send-email-zhiguohong@tencent.com>
You're right, Vlad.
One thing is missing in Eric's fix, NULL dereference is still possible
in br_handle_local_finish. Above is the new version of fix.
On Thu, Sep 12, 2013 at 11:57 PM, Hong Zhiguo <honkiko@gmail.com> wrote:
> not check IFF_BRIDGE_PORT within br_handle_frame
>
> Signed-off-by: Hong Zhiguo <zhiguohong@tencent.com>
> ---
> net/bridge/br_input.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
> index a2fd37e..da4714a 100644
> --- a/net/bridge/br_input.c
> +++ b/net/bridge/br_input.c
> @@ -60,7 +60,7 @@ static int br_pass_frame_up(struct sk_buff *skb)
> int br_handle_frame_finish(struct sk_buff *skb)
> {
> const unsigned char *dest = eth_hdr(skb)->h_dest;
> - struct net_bridge_port *p = br_port_get_rcu(skb->dev);
> + enet_bridge_port *p = rcu_dereference(skb->dev->rx_handler_data);
> struct net_bridge *br;
> struct net_bridge_fdb_entry *dst;
> struct net_bridge_mdb_entry *mdst;
> @@ -143,7 +143,7 @@ drop:
> /* note: already called with rcu_read_lock */
> static int br_handle_local_finish(struct sk_buff *skb)
> {
> - struct net_bridge_port *p = br_port_get_rcu(skb->dev);
> + struct net_bridge_port *p = rcu_dereference(skb->dev->rx_handler_data);
> u16 vid = 0;
>
> br_vlan_get_tag(skb, &vid);
> @@ -173,7 +173,7 @@ rx_handler_result_t br_handle_frame(struct sk_buff **pskb)
> if (!skb)
> return RX_HANDLER_CONSUMED;
>
> - p = br_port_get_rcu(skb->dev);
> + p = rcu_dereference(skb->dev->rx_handler_data);
>
> if (unlikely(is_link_local_ether_addr(dest))) {
> /*
> --
> 1.7.0.4
>
--
best regards
Hong Zhiguo
^ permalink raw reply
* Re: [PATCH net-next] fix NULL pointer dereference in br_handle_frame
From: Eric Dumazet @ 2013-09-12 16:08 UTC (permalink / raw)
To: vyasevic; +Cc: Hong zhi guo, netdev, David Miller, zhiguohong
In-Reply-To: <5231E2C7.1010106@redhat.com>
On Thu, 2013-09-12 at 11:50 -0400, Vlad Yasevich wrote:
> On 09/12/2013 11:24 AM, Eric Dumazet wrote:
> > On Thu, 2013-09-12 at 10:42 -0400, Vlad Yasevich wrote:
> >
> >> Don't all tests for IFF_BRIDGE_PORT on the bridge receive path become
> >> redundant as well?
> >
> > Sure but anyway this part is net-next material
> >
> > Lets fix the bug first.
>
> Seems like the race is possible in net as well. If we just include the
> original patch, then br_add_if() also has to be modified to make sure
> the flag is set properly before netdev_rx_handler_register() is called.
>
> If we remove the need to check the flag during bridge input handling,
> then br_add_if() change is not necessary.
Yep, really testing IFF_BRIDGE_PORT in fast path is currently racy.
rcu protection should be enough.
^ permalink raw reply
* Re: [PATCH net-next] fix NULL pointer dereference in br_handle_frame
From: Eric Dumazet @ 2013-09-12 16:11 UTC (permalink / raw)
To: Hong zhi guo; +Cc: vyasevic, David Miller, netdev, Hong Zhiguo
In-Reply-To: <CAA7+ByXUnTi1+-yreK+q03Cbqg+FM7DTfQ9qXbyRbZ0N508JBQ@mail.gmail.com>
On Fri, 2013-09-13 at 00:06 +0800, Hong zhi guo wrote:
> You're right, Vlad.
> One thing is missing in Eric's fix, NULL dereference is still possible
> in br_handle_local_finish. Above is the new version of fix.
Hey, it was not a 'fix', but a comment on your patch and bridge
defensive programming.
^ permalink raw reply
* Re: [PATCH net-next] fix NULL pointer dereference in br_handle_frame
From: Eric Dumazet @ 2013-09-12 16:12 UTC (permalink / raw)
To: Hong Zhiguo; +Cc: vyasevic, davem, netdev, Hong Zhiguo
In-Reply-To: <1379001428-2727-1-git-send-email-zhiguohong@tencent.com>
On Thu, 2013-09-12 at 23:57 +0800, Hong Zhiguo wrote:
> not check IFF_BRIDGE_PORT within br_handle_frame
>
> Signed-off-by: Hong Zhiguo <zhiguohong@tencent.com>
> ---
No need to hurry. Take the time to write an extensive changelog, and
test the patch !
Thanks
^ permalink raw reply
* Re: [PATCH net-next] fix NULL pointer dereference in br_handle_frame
From: Hong zhi guo @ 2013-09-12 16:18 UTC (permalink / raw)
To: Eric Dumazet; +Cc: vyasevic, David Miller, netdev, Hong Zhiguo
In-Reply-To: <1379002315.24408.19.camel@edumazet-glaptop>
sorry, Eric, maybe I'm using wrong words. Thanks for your review and help.
So you both prefer not testing IFF_BRIDGE_PORT. Let's take the new fix.
On Fri, Sep 13, 2013 at 12:11 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Fri, 2013-09-13 at 00:06 +0800, Hong zhi guo wrote:
>> You're right, Vlad.
>> One thing is missing in Eric's fix, NULL dereference is still possible
>> in br_handle_local_finish. Above is the new version of fix.
>
> Hey, it was not a 'fix', but a comment on your patch and bridge
> defensive programming.
>
>
>
--
best regards
Hong Zhiguo
^ permalink raw reply
* Re: [PATCH net-next v4 1/6] bonding: simplify and use RCU protection for 3ad xmit path
From: Paul E. McKenney @ 2013-09-12 16:17 UTC (permalink / raw)
To: Veaceslav Falico
Cc: Nikolay Aleksandrov, Ding Tianhong, Jay Vosburgh, Andy Gospodarek,
David S. Miller, Netdev
In-Reply-To: <20130907150350.GF26163@redhat.com>
On Sat, Sep 07, 2013 at 05:03:50PM +0200, Veaceslav Falico wrote:
> On Sat, Sep 07, 2013 at 04:45:05PM +0200, Nikolay Aleksandrov wrote:
> >
> >On 09/07/2013 04:20 PM, Veaceslav Falico wrote:
> >>On Fri, Sep 06, 2013 at 03:28:07PM +0800, Ding Tianhong wrote:
> ...snip...
> >>diff --git a/include/linux/rculist.h b/include/linux/rculist.h
> >>index f4b1001..37b49d1 100644
> >>--- a/include/linux/rculist.h
> >>+++ b/include/linux/rculist.h
> >>@@ -23,6 +23,7 @@
> >> * way, we must not access it directly
> >> */
> >> #define list_next_rcu(list) (*((struct list_head __rcu
> >>**)(&(list)->next)))
> >>+#define list_prev_rcu(list) (*((struct list_head __rcu
> >>**)(&(list)->prev)))
> >>
> >> /*
> >> * Insert a new entry between two known consecutive entries.
> >>@@ -271,6 +272,12 @@ static inline void list_splice_init_rcu(struct
> >>list_head *list,
> >> likely(__ptr != __next) ? container_of(__next, type, member) : NULL; \
> >> })
> >>
> >>+#define list_last_or_null_rcu(ptr, type, member) \
> >>+ ({struct list_head *__ptr = (ptr); \
> >>+ struct list_head __rcu *__last = list_prev_rcu(__ptr); \
> >>+ likely(__ptr != __last) ? container_of(__prev, type, member) : NULL; \
> >>+ })
> >>+
> >Hi,
> >Actually I don't think you can dereference ->prev and use the standard
> >list_del_rcu because it guarantees only the ->next ptr will be valid and
> >->prev is set to LIST_POISON2.
> >IMO, you'll need something like this: https://lkml.org/lkml/2012/7/25/193
> >with the bidir_del and all that.
>
> Yeah, right, my bad - we can rely only on the ->next pointer, indeed,
> missed that part. RCU is hard :).
>
> So it'll be a lot harder to implement bond_last_slave_rcu() in a
> 'straightforward' approach.
>
> I'd rather go in the opposite direction here - i.e. drop the 'reverse'
> traversal completely, and all the use cases for bond_last_slave_rcu(). I've
> got some patches already - http://patchwork.ozlabs.org/patch/272076/ doing
> that, and hopefully will remove the whole 'backword' traversal completely
> in the future.
If it is important, it would be possible to create an RCU-protected
linked list that allowed readers to go in both directions -- mostly just
leave out the poisoning of the ->prev pointers. We do -not- want to
do this for the normal RCU-protected linked lists because the poisoning
does find bugs.
However, you would have to be quite careful with a bi-directional
RCU-protected linked list, because p->next->prev will not necessarily
be equal to p, for all the reasons that you guys listed out earlier in
this thread. So be very careful what you wish for! ;-)
Thanx, Paul
> >But in any case I complete agree with Veaceslav here. Read all the
> >documentation carefully :-)
> >
> >Cheers,
> >Nik
> >
> >> /**
> >> * list_for_each_entry_rcu - iterate over rcu list of given type
> >> * @pos: the type * to use as a loop cursor.
> >>------- END OF PATCH ------
> >>
> >>Anyway, it's up to you.
> >>
> >>Hope that helps.
> >
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH net-next] fix NULL pointer dereference in br_handle_frame
From: Eric Dumazet @ 2013-09-12 16:19 UTC (permalink / raw)
To: Hong zhi guo; +Cc: vyasevic, David Miller, netdev, Hong Zhiguo
In-Reply-To: <CAA7+ByUy5jL2hpkN+GBDRCXh_GM7qnS58iZvgp1RxZmoAqj+bQ@mail.gmail.com>
On Fri, 2013-09-13 at 00:18 +0800, Hong zhi guo wrote:
> sorry, Eric, maybe I'm using wrong words. Thanks for your review and help.
> So you both prefer not testing IFF_BRIDGE_PORT. Let's take the new fix.
Please do not top post on netdev.
Thanks
^ permalink raw reply
* [PATCH net-next v3] Don't destroy the netdev until the vif is shut down
From: Paul Durrant @ 2013-09-12 16:19 UTC (permalink / raw)
To: xen-devel, netdev; +Cc: Paul Durrant, David Vrabel, Wei Liu, Ian Campbell
Without this patch, if a frontend cycles through states Closing
and Closed (which Windows frontends need to do) then the netdev
will be destroyed and requires re-invocation of hotplug scripts
to restore state before the frontend can move to Connected. Thus
when udev is not in use the backend gets stuck in InitWait.
With this patch, the netdev is left alone whilst the backend is
still online and is only de-registered and freed just prior to
destroying the vif (which is also nicely symmetrical with the
netdev allocation and registration being done during probe) so
no re-invocation of hotplug scripts is required.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Cc: David Vrabel <david.vrabel@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
---
v2:
- Modify netback_remove() - bug only seemed to manifest with linux guest
v3:
- Move __module_get() and module_put() calls
drivers/net/xen-netback/common.h | 1 +
drivers/net/xen-netback/interface.c | 25 +++++++++----------------
drivers/net/xen-netback/xenbus.c | 17 ++++++++++++-----
3 files changed, 22 insertions(+), 21 deletions(-)
diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index a197743..5715318 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -184,6 +184,7 @@ int xenvif_connect(struct xenvif *vif, unsigned long tx_ring_ref,
unsigned long rx_ring_ref, unsigned int tx_evtchn,
unsigned int rx_evtchn);
void xenvif_disconnect(struct xenvif *vif);
+void xenvif_free(struct xenvif *vif);
int xenvif_xenbus_init(void);
void xenvif_xenbus_fini(void);
diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
index 625c6f4..6b1096d 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -353,6 +353,9 @@ struct xenvif *xenvif_alloc(struct device *parent, domid_t domid,
}
netdev_dbg(dev, "Successfully created xenvif\n");
+
+ __module_get(THIS_MODULE);
+
return vif;
}
@@ -366,8 +369,6 @@ int xenvif_connect(struct xenvif *vif, unsigned long tx_ring_ref,
if (vif->tx_irq)
return 0;
- __module_get(THIS_MODULE);
-
err = xenvif_map_frontend_rings(vif, tx_ring_ref, rx_ring_ref);
if (err < 0)
goto err;
@@ -452,12 +453,6 @@ void xenvif_carrier_off(struct xenvif *vif)
void xenvif_disconnect(struct xenvif *vif)
{
- /* Disconnect funtion might get called by generic framework
- * even before vif connects, so we need to check if we really
- * need to do a module_put.
- */
- int need_module_put = 0;
-
if (netif_carrier_ok(vif->dev))
xenvif_carrier_off(vif);
@@ -468,23 +463,21 @@ void xenvif_disconnect(struct xenvif *vif)
unbind_from_irqhandler(vif->tx_irq, vif);
unbind_from_irqhandler(vif->rx_irq, vif);
}
- /* vif->irq is valid, we had a module_get in
- * xenvif_connect.
- */
- need_module_put = 1;
}
if (vif->task)
kthread_stop(vif->task);
+ xenvif_unmap_frontend_rings(vif);
+}
+
+void xenvif_free(struct xenvif *vif)
+{
netif_napi_del(&vif->napi);
unregister_netdev(vif->dev);
- xenvif_unmap_frontend_rings(vif);
-
free_netdev(vif->dev);
- if (need_module_put)
- module_put(THIS_MODULE);
+ module_put(THIS_MODULE);
}
diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index 1fe48fe3..a53782e 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -42,7 +42,7 @@ static int netback_remove(struct xenbus_device *dev)
if (be->vif) {
kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE);
xenbus_rm(XBT_NIL, dev->nodename, "hotplug-status");
- xenvif_disconnect(be->vif);
+ xenvif_free(be->vif);
be->vif = NULL;
}
kfree(be);
@@ -213,9 +213,18 @@ static void disconnect_backend(struct xenbus_device *dev)
{
struct backend_info *be = dev_get_drvdata(&dev->dev);
+ if (be->vif)
+ xenvif_disconnect(be->vif);
+}
+
+static void destroy_backend(struct xenbus_device *dev)
+{
+ struct backend_info *be = dev_get_drvdata(&dev->dev);
+
if (be->vif) {
+ kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE);
xenbus_rm(XBT_NIL, dev->nodename, "hotplug-status");
- xenvif_disconnect(be->vif);
+ xenvif_free(be->vif);
be->vif = NULL;
}
}
@@ -246,14 +255,11 @@ static void frontend_changed(struct xenbus_device *dev,
case XenbusStateConnected:
if (dev->state == XenbusStateConnected)
break;
- backend_create_xenvif(be);
if (be->vif)
connect(be);
break;
case XenbusStateClosing:
- if (be->vif)
- kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE);
disconnect_backend(dev);
xenbus_switch_state(dev, XenbusStateClosing);
break;
@@ -262,6 +268,7 @@ static void frontend_changed(struct xenbus_device *dev,
xenbus_switch_state(dev, XenbusStateClosed);
if (xenbus_dev_is_online(dev))
break;
+ destroy_backend(dev);
/* fall through if not online */
case XenbusStateUnknown:
device_unregister(&dev->dev);
--
1.7.10.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox