* Re: [PATCH] vhost/scsi: use vmalloc for order-10 allocation
From: Sergei Shtylyov @ 2013-09-17 18:14 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: kvm, netdev, linux-kernel, virtualization, Dan Aloni
In-Reply-To: <1379401998-5131-1-git-send-email-mst@redhat.com>
Hello.
On 09/17/2013 11:21 AM, Michael S. Tsirkin wrote:
> As vhost scsi device struct is large, if the device is
> created on a busy system, kzalloc() might fail, so this patch does a
> fallback to vzalloc().
> As vmalloc() adds overhead on data-path, add __GFP_REPEAT
> to kzalloc() flags to do this fallback only when really needed.
> Reported-by: Dan Aloni <alonid@stratoscale.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> I put this on my vhost fixes branch, intend to merge for 3.12.
> Dan, could you please confirm this works for you?
> drivers/vhost/scsi.c | 41 +++++++++++++++++++++++++++--------------
> 1 file changed, 27 insertions(+), 14 deletions(-)
> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> index 4b79a1f..2c30bb0 100644
> --- a/drivers/vhost/scsi.c
> +++ b/drivers/vhost/scsi.c
> @@ -1373,21 +1373,30 @@ static int vhost_scsi_set_features(struct vhost_scsi *vs, u64 features)
> return 0;
> }
>
> +static void vhost_scsi_free(struct vhost_scsi *vs)
> +{
> + if (is_vmalloc_addr(vs))
> + vfree(vs);
> + else
> + kfree(vs);
Indent with the tabs ISO spaces, please.
> +}
> +
> static int vhost_scsi_open(struct inode *inode, struct file *f)
> {
> struct vhost_scsi *vs;
> struct vhost_virtqueue **vqs;
> - int r, i;
> + int r = -ENOMEM, i;
>
> - vs = kzalloc(sizeof(*vs), GFP_KERNEL);
> - if (!vs)
> - return -ENOMEM;
> + vs = kzalloc(sizeof(*vs), GFP_KERNEL | __GFP_NOWARN | __GFP_REPEAT);
Indent here with a tab, please.
> + if (!vs) {
> + vs = vzalloc(sizeof(*vs));
> + if (!vs)
> + goto err_vs;
> + }
WBR, Sergei
^ permalink raw reply
* Re: [PATCH -net] netpoll: fix NULL pointer dereference in netpoll_cleanup
From: Nikolay Aleksandrov @ 2013-09-17 18:06 UTC (permalink / raw)
To: netdev; +Cc: davem
In-Reply-To: <52386911.2010504@redhat.com>
On 09/17/2013 04:37 PM, Nikolay Aleksandrov wrote:
> commit 7a163bfb7ce50895bbe67300ea610d31b9c09230
> ("netconsole: avoid a crash with multiple sysfs writers")
I feel like I didn't explain this one well. The above commit actually
tries to fix the same issue AFAICT, and it can be reverted if/once my fix
is accepted, but I think to remove only the locking in the netconsole
netdev notifier to avoid the deadlock because the mutex lock is useful for
fixing a third bug in netconsole, that I intend to take care of once this
is sorted out.
So basically my next fix that takes care of the deadlock is dependent on
this patch, and I'll wait to see the feedback, if it gets accepted I'll
post the follow-up that takes care of the deadlock.
Cheers,
Nik
^ permalink raw reply
* Re: [CFT][PATCH] net: Delay default_device_exit_batch until no devices are unregistering
From: Francesco Ruggeri @ 2013-09-17 17:14 UTC (permalink / raw)
To: Eric W. Biederman
Cc: David S. Miller, Eric Dumazet, Jiri Pirko, Alexander Duyck,
Cong Wang, netdev
In-Reply-To: <87mwnb949b.fsf@xmission.com>
>
> If you could test this patch perhaps refine it I think we are almost at
> a final point of fixing this.
>
I will.
> Just to be clear my reason for prefering this approach is that because
> it adds no extra wait points (we already wait for the rtnl_lock), the
> logic is unconditional and explicit and not hidden in the loopback
> device's reference count. Which should allow anyone reading the code
> to discover and understand this guarantee. Although a big fat comment
> in default_device_exit_batch that we are guaranteeing we don't allow
> the network namespace to exit while there are still network devices in
> it (or something to that effect) is probably appropriate.
I agree, this approach is cleaner than overloading loopback_dev's
refcount as in my original patch.
I will let you know how my tests go over the next few days.
Thanks,
Francesco
>
> Eric
>
^ permalink raw reply
* RE: [PATCH v4] Don't destroy the netdev until the vif is shut down
From: Paul Durrant @ 2013-09-17 16:45 UTC (permalink / raw)
To: Paul Durrant, xen-devel@lists.xen.org, netdev@vger.kernel.org
Cc: David Vrabel, Wei Liu, Ian Campbell
In-Reply-To: <1379432723-24384-1-git-send-email-paul.durrant@citrix.com>
> -----Original Message-----
> From: Paul Durrant [mailto:paul.durrant@citrix.com]
> Sent: 17 September 2013 16:45
> To: xen-devel@lists.xen.org; netdev@vger.kernel.org
> Cc: Paul Durrant; David Vrabel; Wei Liu; Ian Campbell
> Subject: [PATCH v4] Don't destroy the netdev until the vif is shut down
>
Apologies for the loss of the net-next prefix here. I'll re-send with the prefix.
Paul
> 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
>
> v4:
> - Clear tx_irq in xenvif_disconnect() to make sure that a subsequent
> call to xenvif_connect() doesn't get nop-ed.
>
> drivers/net/xen-netback/common.h | 1 +
> drivers/net/xen-netback/interface.c | 26 ++++++++++----------------
> drivers/net/xen-netback/xenbus.c | 17 ++++++++++++-----
> 3 files changed, 23 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..0465e0f 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,22 @@ 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;
> + vif->tx_irq = 0;
> }
>
> 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
* [PATCH net-next v4] Don't destroy the netdev until the vif is shut down
From: Paul Durrant @ 2013-09-17 16:46 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
v4:
- Clear tx_irq in xenvif_disconnect() to make sure that a subsequent
call to xenvif_connect() doesn't get nop-ed.
drivers/net/xen-netback/common.h | 1 +
drivers/net/xen-netback/interface.c | 26 ++++++++++----------------
drivers/net/xen-netback/xenbus.c | 17 ++++++++++++-----
3 files changed, 23 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..0465e0f 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,22 @@ 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;
+ vif->tx_irq = 0;
}
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
* Re: [PATCHv2 net] xen-netback: count number required slots for an skb more carefully
From: David Vrabel @ 2013-09-17 16:41 UTC (permalink / raw)
To: Ian Campbell
Cc: David Miller, xen-devel, konrad.wilk, boris.ostrovsky, netdev
In-Reply-To: <1379426727.11304.103.camel@hastur.hellion.org.uk>
On 17/09/13 15:05, Ian Campbell wrote:
> On Thu, 2013-09-12 at 23:23 -0400, David Miller wrote:
>> I assume you want this queued up for -stable,
>
> I think so, David V -- do you know how far back this goes?
Stable please, but only back to 3.10.
I suspect (but never had time to confirm) that this fixes a regression
introduced by "xen/netback: Calculate the number of SKB slots required
correctly" e26b203e which was added in 3.6.
>> and can you check if
>> there is any non-trivial backporting for earlier kernels?
>
> Another one I hope David can answer...
Backport to 3.10 "just works".
David
^ permalink raw reply
* Re: [PATCH net-next] bridge: change the order of actions in addif/delif
From: Stephen Hemminger @ 2013-09-17 16:24 UTC (permalink / raw)
To: Hong Zhiguo; +Cc: netdev, davem, eric.dumazet, vyasevic, Hong Zhiguo
In-Reply-To: <1379403883-16219-1-git-send-email-zhiguohong@tencent.com>
On Tue, 17 Sep 2013 15:44:43 +0800
Hong Zhiguo <honkiko@gmail.com> wrote:
> - /* Device is already being bridged */
> - if (br_port_exists(dev))
> + /* Device is already being bridged or registered with other handler */
> + if (br_port_exists(dev) || dev->rx_handler)
> return -EBUSY;
Direct access to dev->rx_handler should be avoided.
The error check should be in netdev_rx_handler_register.
^ permalink raw reply
* [PATCH v4] Don't destroy the netdev until the vif is shut down
From: Paul Durrant @ 2013-09-17 15:45 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
v4:
- Clear tx_irq in xenvif_disconnect() to make sure that a subsequent
call to xenvif_connect() doesn't get nop-ed.
drivers/net/xen-netback/common.h | 1 +
drivers/net/xen-netback/interface.c | 26 ++++++++++----------------
drivers/net/xen-netback/xenbus.c | 17 ++++++++++++-----
3 files changed, 23 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..0465e0f 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,22 @@ 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;
+ vif->tx_irq = 0;
}
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
* Re: Why we discard all rtt samples when only some of the acked skbs have been retransmited in processing ack?
From: Yuchung Cheng @ 2013-09-17 14:53 UTC (permalink / raw)
To: Neal Cardwell; +Cc: Eric Dumazet, LovelyLich, Netdev
In-Reply-To: <CADVnQykjm8nxVYzFu7=RfdcOH-cdARarSoRyBwj9Rp0m-+VKEw@mail.gmail.com>
On Tue, Sep 17, 2013 at 6:31 AM, Neal Cardwell <ncardwell@google.com> wrote:
> On Tue, Sep 17, 2013 at 1:11 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> On Tue, 2013-09-17 at 12:01 +0800, LovelyLich wrote:
>>> Hi Eric,
>>>
>>> In tcp_clean_rtx_queue(), we set the flag FLAG_RETRANS_DATA_ACKED when we
>>>
>>> encounter one ever retransmited skb A. But if there is one( or more) skb B
>>>
>>> after this retransmited skb, and we calculate the rtt for skb B. The question
>>>
>>> is because we have set the flag FLAG_RETRANS_DATA_ACKED, and we will just
>>>
>>> return in tcp_ack_no_tstamp() !
>>>
>>> Two questions:
>>>
>>> 1. if we will just ignore all packets in this ack, we do not need to calculate
>>>
>>> skb B's rtt sample.
>>>
>>> 2. what I want to know, even if A's rtt sample is not reliable, but B's rtt
>>>
>>> sample can be trusted. Why we discard it ?
>>>
>>>
>>>
>>> Thanks in advanced.
>>>
>>
>> Good point !
>>
>> Yuchung, what do you think of following patch ?
>>
>> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
>> index 25a89ea..7f12b96 100644
>> --- a/net/ipv4/tcp_input.c
>> +++ b/net/ipv4/tcp_input.c
>> @@ -2971,7 +2971,7 @@ static int tcp_clean_rtx_queue(struct sock *sk, int prior_fackets,
>> struct sk_buff *skb;
>> u32 now = tcp_time_stamp;
>> int fully_acked = true;
>> - int flag = 0;
>> + int flag = FLAG_RETRANS_DATA_ACKED;
>> u32 pkts_acked = 0;
>> u32 reord = tp->packets_out;
>> u32 prior_sacked = tp->sacked_out;
>> @@ -3002,7 +3002,6 @@ static int tcp_clean_rtx_queue(struct sock *sk, int prior_fackets,
>> if (sacked & TCPCB_RETRANS) {
>> if (sacked & TCPCB_SACKED_RETRANS)
>> tp->retrans_out -= acked_pcount;
>> - flag |= FLAG_RETRANS_DATA_ACKED;
>> } else {
>> ca_seq_rtt = now - scb->when;
>> last_ackt = skb->tstamp;
>> @@ -3013,6 +3012,7 @@ static int tcp_clean_rtx_queue(struct sock *sk, int prior_fackets,
>> reord = min(pkts_acked, reord);
>> if (!after(scb->end_seq, tp->high_seq))
>> flag |= FLAG_ORIG_SACK_ACKED;
>> + flag &= ~FLAG_RETRANS_DATA_ACKED;
>> }
>>
>> if (sacked & TCPCB_SACKED_ACKED)
>
> I think the existing logic is better than the patch. If we get a
> cumulative ACK that covers a retransmitted packet, then any RTT sample
> we try to extract is suspect, and likely to be at least 2x too high.
>
> Consider the following common scenario:
>
> t=0: send pkts 1, 2, 3, 4
> t=1*RTT: receive dupack with SACK for pkts 2,3,4; fast retransmit pkt 1
> t=2*RTT: receive cumulative ack for all pkts through 4
>
> With the existing logic, because the ACK we get at t=2*RTT covers the
> retransmitted pkt 1, we do not attempt to take an RTT sample.
>
> With that proposed patch, when we get the ACK at t=2*RTT we see that
> there are non-retransmitted pkts 2,3,4 being ACKed, so we clear the
> FLAG_RETRANS_DATA_ACKED bit and take an RTT sample of 2*RTT. But this
> is 2x too big, and will distort our RTT sample.
Yes I completely agree with Neal's analysis. In fact I've considered
the proposed change when I implement the patch quoted below, and later
realized it actually breaks Karn's algorithm: to properly take an RTT
sample w/o ambiguity, all, not just some, of the packets cumulatively
acked must not been retransmitted.
>
> Note that with Yuchung's recent patch to gather RTT samples from
> SACKed packets (59c9af4234b0c21a1ed05cf65bf014d0c1a67bfd "tcp: measure
> RTT from new SACK"), we will already be extracting essentially all the
> RTT samples that we possibly can out of such scenarios with
> retransmitted packets (for OSes that support SACK, which is basically
> everyone). In the example above, Yuchung's new SACK-based RTT scheme
> will correctly take RTT samples at t=1*RTT for the SACKed packets.
>
> neal
^ permalink raw reply
* Re: [PATCH -net] netpoll: fix NULL pointer dereference in netpoll_cleanup
From: Nikolay Aleksandrov @ 2013-09-17 14:37 UTC (permalink / raw)
To: netdev; +Cc: davem
In-Reply-To: <1379427155-8561-1-git-send-email-nikolay@redhat.com>
On 09/17/2013 04:12 PM, Nikolay Aleksandrov wrote:
> I've been hitting a NULL ptr deref while using netconsole because the
> np->dev check and the pointer manipulation in netpoll_cleanup are done
> without rtnl and the following sequence happens when having a netconsole
> over a vlan and we remove the vlan while disabling the netconsole:
> CPU 1 CPU2
> removes vlan and calls the notifier
> enters store_enabled(), calls
> netdev_cleanup which checks np->dev
> and then waits for rtnl
> executes the netconsole netdev
> release notifier making np->dev
> == NULL and releases rtnl
> continues to dereference a member of
> np->dev which at this point is == NULL
>
> Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
> ---
Just FYI there seems to be a deadlock in netconsole as well:
rtnl -> nt->mutex in the notifier coupled with
nt->mutex -> rtnl in store_enabled()
I can re-post a patchset that fixes these together, because after this is
applied the NULL pointer dereference is not hit, but the deadlock is easily hit.
The deadlock was introduced in commit 7a163bfb7ce50895bbe67300ea610d31b9c09230
("netconsole: avoid a crash with multiple sysfs writers").
Nik
^ permalink raw reply
* Re: [PATCH net] tcp: fix RTO calculated from cached RTT
From: Yuchung Cheng @ 2013-09-17 14:32 UTC (permalink / raw)
To: Neal Cardwell; +Cc: David Miller, netdev, Eric Dumazet
In-Reply-To: <1379382260-29303-1-git-send-email-ncardwell@google.com>
On Mon, Sep 16, 2013 at 6:44 PM, Neal Cardwell <ncardwell@google.com> wrote:
> Commit 1b7fdd2ab5852 ("tcp: do not use cached RTT for RTT estimation")
> did not correctly account for the fact that crtt is the RTT shifted
> left 3 bits. Fix the calculation to consistently reflect this fact.
>
> Signed-off-by: Neal Cardwell <ncardwell@google.com>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Yuchung Cheng <ycheng@google.com>
> ---
> net/ipv4/tcp_metrics.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
Acked-By: Yuchung Cheng <ycheng@google.com>
Thanks for discovering and fixing it!
>
> diff --git a/net/ipv4/tcp_metrics.c b/net/ipv4/tcp_metrics.c
> index 4a22f3e..52f3c6b 100644
> --- a/net/ipv4/tcp_metrics.c
> +++ b/net/ipv4/tcp_metrics.c
> @@ -502,7 +502,9 @@ reset:
> * ACKs, wait for troubles.
> */
> if (crtt > tp->srtt) {
> - inet_csk(sk)->icsk_rto = crtt + max(crtt >> 2, tcp_rto_min(sk));
> + /* Set RTO like tcp_rtt_estimator(), but from cached RTT. */
> + crtt >>= 3;
> + inet_csk(sk)->icsk_rto = crtt + max(2 * crtt, tcp_rto_min(sk));
> } else if (tp->srtt == 0) {
> /* RFC6298: 5.7 We've failed to get a valid RTT sample from
> * 3WHS. This is most likely due to retransmission,
> --
> 1.8.4
>
^ permalink raw reply
* [PATCH -net] netpoll: fix NULL pointer dereference in netpoll_cleanup
From: Nikolay Aleksandrov @ 2013-09-17 14:12 UTC (permalink / raw)
To: netdev; +Cc: davem
I've been hitting a NULL ptr deref while using netconsole because the
np->dev check and the pointer manipulation in netpoll_cleanup are done
without rtnl and the following sequence happens when having a netconsole
over a vlan and we remove the vlan while disabling the netconsole:
CPU 1 CPU2
removes vlan and calls the notifier
enters store_enabled(), calls
netdev_cleanup which checks np->dev
and then waits for rtnl
executes the netconsole netdev
release notifier making np->dev
== NULL and releases rtnl
continues to dereference a member of
np->dev which at this point is == NULL
Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
net/core/netpoll.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 2c637e9..569a185 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -1284,15 +1284,15 @@ EXPORT_SYMBOL_GPL(__netpoll_free_async);
void netpoll_cleanup(struct netpoll *np)
{
- if (!np->dev)
- return;
-
rtnl_lock();
+ if (!np->dev) {
+ rtnl_unlock();
+ return;
+ }
__netpoll_cleanup(np);
- rtnl_unlock();
-
dev_put(np->dev);
np->dev = NULL;
+ rtnl_unlock();
}
EXPORT_SYMBOL(netpoll_cleanup);
--
1.8.1.4
^ permalink raw reply related
* Re: [PATCHv2 net] xen-netback: count number required slots for an skb more carefully
From: Ian Campbell @ 2013-09-17 14:05 UTC (permalink / raw)
To: David Miller
Cc: david.vrabel, xen-devel, konrad.wilk, boris.ostrovsky, netdev
In-Reply-To: <20130912.232328.1199260755421139102.davem@davemloft.net>
On Thu, 2013-09-12 at 23:23 -0400, David Miller wrote:
> I assume you want this queued up for -stable,
I think so, David V -- do you know how far back this goes?
> and can you check if
> there is any non-trivial backporting for earlier kernels?
Another one I hope David can answer...
Ian.
^ permalink raw reply
* Re: [PATCH v3 net-next 21/27] net: add a function to get the next private
From: Veaceslav Falico @ 2013-09-17 13:55 UTC (permalink / raw)
To: Ben Hutchings
Cc: netdev, jiri, David S. Miller, Eric Dumazet, Alexander Duyck
In-Reply-To: <1379382601.23881.36.camel@deadeye.wl.decadent.org.uk>
On Tue, Sep 17, 2013 at 02:50:01AM +0100, Ben Hutchings wrote:
>On Tue, 2013-09-17 at 02:46 +0200, Veaceslav Falico wrote:
>> It searches for the provided private and returns the next one. If private
>> is not found or next list element is list head - returns NULL.
>
>This is going to take linear time, which is probably OK for a bond that
>has only a very few devices. But it would likely be a really bad idea
>for, say, a bridge device that could have tens or hundreds of lower
>devices. So it's not a generically useful function.
Indeed, you're right. I've tried searching or trying to figure out why
others could need it - with no luck. It's really bonding-specific. And, in
any case, if there will be more users of it - it can be changed in the
future.
>
>I think the bonding driver can implement this:
>
>[...]
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -5055,6 +5055,33 @@ void *netdev_lower_dev_get_private(struct net_device *dev,
>> }
>> EXPORT_SYMBOL(netdev_lower_dev_get_private);
>>
>> +/* netdev_lower_dev_get_next_private - return the ->private of the list
>> + * element whos ->private == private.
>> + * @dev - device to search
>> + * @private - private pointer to search for.
>> + *
>> + * Returns the next ->private pointer, if ->next is not head and private is
>> + * found.
>> + */
>> +extern void *netdev_lower_dev_get_next_private(struct net_device *dev,
>> + void *private)
>> +{
>> + struct netdev_adjacent *lower;
>> +
>> + list_for_each_entry(lower, &dev->adj_list.lower, list) {
>> + if (lower->private == private) {
>> + lower = list_entry(lower->list.next,
>> + struct netdev_adjacent, list);
>> + if (&lower->list == &dev->adj_list.lower)
>> + return NULL;
>> + return lower->private;
>> + }
>> + }
>> +
>> + return NULL;
>> +}
>> +EXPORT_SYMBOL(netdev_lower_dev_get_next_private);
>
>using only the functions already exported:
>
>static void *__bond_next_slave(struct net_device *dev, void *private)
>{
> struct list_head *iter;
> struct net_device *lower;
> bool found = false;
>
> netdev_for_each_lower_dev(dev, lower, iter) {
> if (found)
> return netdev_adjacent_get_private(iter);
> if (netdev_adjacent_get_private(iter) == private)
> found = true;
> }
>
> return NULL;
>}
>
>(not that I've tested it :-).
Yep, I'll think of something like that and send it in the next version,
dropping the current netdev_ variant.
Thanks a lot!
>
>Ben.
>
>> +
>> static void dev_change_rx_flags(struct net_device *dev, int flags)
>> {
>> const struct net_device_ops *ops = dev->netdev_ops;
>
>--
>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 v3 0/6] ipv6: Do route updating for redirect in ndisc layer
From: Hannes Frederic Sowa @ 2013-09-17 13:52 UTC (permalink / raw)
To: Duan Jiong
Cc: Daniel Borkmann, Duan Jiong, davem, netdev,
linux-sctp@vger.kernel.org
In-Reply-To: <523710CC.1090404@gmail.com>
On Mon, Sep 16, 2013 at 10:08:12PM +0800, Duan Jiong wrote:
> 于 2013/9/16 20:22, Daniel Borkmann 写道:
> > On 09/16/2013 01:47 PM, Duan Jiong wrote:
> >> From: Duan Jiong <duanj.fnst@cn.fujitsu.com>
> >>
> >> the ip6_redirect() could be replaced with
> >> ip6_redirect_no_header(), we could always use ip6_redirect()
> >> for route updating in ndisc layer and use the data of the
> >> redirected header option just for finding the socket to be
> >> notified and then notify user in protocols' err_handler.
> > If I get this right, it seems to me that this patchset actually consists of two
> > different kind of changes:
> >
> > 1) Not notifying user space on ICMP redirects (net material)
> > 2) Simplify code for updating route in ndisc layer instead of error handlers (net-next)
> >
> > Also, you do the *actual* change in the very last patch, which means that from
> > patch 1 to 5 we're in an inconsistent and buggy state unless we also apply patch
> > number 6. It should actually be the other way around, that you first do the actual
> > change and then migrate users (also commit messages are quite terse).
>
> I make the patch set on net tree, not on net-next. Maybe those
> things should be done in two patch sets.
I have thought about going the other direction. Just make it one patch
and update the whole logic atomically in the git tree. So we won't have
any partial logic updates in the tree.
Please also rebase your series onto net from today, because Daniel's
patch regarding updating sk->sk_err in sctp went in yesterday.
> > Moreover, just looking at the SCTP part (sctp_err_lookup() function) ...
> >
> > /* RFC 4960, Appendix C. ICMP Handling
> > *
> > * ICMP6) An implementation MUST validate that the Verification Tag
> > * contained in the ICMP message matches the Verification Tag of
> > * the peer. If the Verification Tag is not 0 and does NOT
> > * match, discard the ICMP message. If it is 0 and the ICMP
> > * message contains enough bytes to verify that the chunk type is
> > * an INIT chunk and that the Initiate Tag matches the tag of the
> > * peer, continue with ICMP7. If the ICMP message is too short
> > * or the chunk type or the Initiate Tag does not match, silently
> > * discard the packet.
> > */
> >
> > ... it seems to me that we would simply ignore such RFC requirements with
> > your patch for the sctp_v6_err() part.
> >
> > Care to elaborate? ;-)
>
> Sorry, i didn't notice that.
>
> According to the RFC requirements, it suggests that we can't update
> route for redirect in ndisc layer before calling into sctp_err_lookup(),
> because we must verify the ICMP Message. Now maybe we update route for
> redirect in ndisc layer is wrong.
>
> Do you have any idea?
IMO updating of routes in ndisc layer is fine. We already accept redirects
(and also change routes) for sctp connections where the redirect packet does
not contain any tag. Also it is debatable if redirects are counted as
icmp packets or merely just belong to the kind of neighbour discovery
packets which just use icmp framing (and so the sctp rfc would not say
anything about them).
As Daniel already said, it would be better to update the commit message
to clarify the reasons why this is ok (with some pointers to RFCs).
Greetings,
Hannes
^ permalink raw reply
* Re: Why we discard all rtt samples when only some of the acked skbs have been retransmited in processing ack?
From: Neal Cardwell @ 2013-09-17 13:31 UTC (permalink / raw)
To: Eric Dumazet; +Cc: LovelyLich, Yuchung Cheng, Netdev
In-Reply-To: <1379394682.29845.2.camel@edumazet-glaptop>
On Tue, Sep 17, 2013 at 1:11 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Tue, 2013-09-17 at 12:01 +0800, LovelyLich wrote:
>> Hi Eric,
>>
>> In tcp_clean_rtx_queue(), we set the flag FLAG_RETRANS_DATA_ACKED when we
>>
>> encounter one ever retransmited skb A. But if there is one( or more) skb B
>>
>> after this retransmited skb, and we calculate the rtt for skb B. The question
>>
>> is because we have set the flag FLAG_RETRANS_DATA_ACKED, and we will just
>>
>> return in tcp_ack_no_tstamp() !
>>
>> Two questions:
>>
>> 1. if we will just ignore all packets in this ack, we do not need to calculate
>>
>> skb B's rtt sample.
>>
>> 2. what I want to know, even if A's rtt sample is not reliable, but B's rtt
>>
>> sample can be trusted. Why we discard it ?
>>
>>
>>
>> Thanks in advanced.
>>
>
> Good point !
>
> Yuchung, what do you think of following patch ?
>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 25a89ea..7f12b96 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -2971,7 +2971,7 @@ static int tcp_clean_rtx_queue(struct sock *sk, int prior_fackets,
> struct sk_buff *skb;
> u32 now = tcp_time_stamp;
> int fully_acked = true;
> - int flag = 0;
> + int flag = FLAG_RETRANS_DATA_ACKED;
> u32 pkts_acked = 0;
> u32 reord = tp->packets_out;
> u32 prior_sacked = tp->sacked_out;
> @@ -3002,7 +3002,6 @@ static int tcp_clean_rtx_queue(struct sock *sk, int prior_fackets,
> if (sacked & TCPCB_RETRANS) {
> if (sacked & TCPCB_SACKED_RETRANS)
> tp->retrans_out -= acked_pcount;
> - flag |= FLAG_RETRANS_DATA_ACKED;
> } else {
> ca_seq_rtt = now - scb->when;
> last_ackt = skb->tstamp;
> @@ -3013,6 +3012,7 @@ static int tcp_clean_rtx_queue(struct sock *sk, int prior_fackets,
> reord = min(pkts_acked, reord);
> if (!after(scb->end_seq, tp->high_seq))
> flag |= FLAG_ORIG_SACK_ACKED;
> + flag &= ~FLAG_RETRANS_DATA_ACKED;
> }
>
> if (sacked & TCPCB_SACKED_ACKED)
I think the existing logic is better than the patch. If we get a
cumulative ACK that covers a retransmitted packet, then any RTT sample
we try to extract is suspect, and likely to be at least 2x too high.
Consider the following common scenario:
t=0: send pkts 1, 2, 3, 4
t=1*RTT: receive dupack with SACK for pkts 2,3,4; fast retransmit pkt 1
t=2*RTT: receive cumulative ack for all pkts through 4
With the existing logic, because the ACK we get at t=2*RTT covers the
retransmitted pkt 1, we do not attempt to take an RTT sample.
With that proposed patch, when we get the ACK at t=2*RTT we see that
there are non-retransmitted pkts 2,3,4 being ACKed, so we clear the
FLAG_RETRANS_DATA_ACKED bit and take an RTT sample of 2*RTT. But this
is 2x too big, and will distort our RTT sample.
Note that with Yuchung's recent patch to gather RTT samples from
SACKed packets (59c9af4234b0c21a1ed05cf65bf014d0c1a67bfd "tcp: measure
RTT from new SACK"), we will already be extracting essentially all the
RTT samples that we possibly can out of such scenarios with
retransmitted packets (for OSes that support SACK, which is basically
everyone). In the example above, Yuchung's new SACK-based RTT scheme
will correctly take RTT samples at t=1*RTT for the SACKed packets.
neal
^ permalink raw reply
* Re: [PATCH v3 net-next 07/27] net: add for_each iterators through neighbour lower link's private
From: Veaceslav Falico @ 2013-09-17 12:29 UTC (permalink / raw)
To: Eric Dumazet
Cc: Ben Hutchings, netdev, jiri, David S. Miller, Eric Dumazet,
Alexander Duyck
In-Reply-To: <1379418182.3457.2.camel@edumazet-glaptop>
On Tue, Sep 17, 2013 at 04:43:02AM -0700, Eric Dumazet wrote:
>On Tue, 2013-09-17 at 09:36 +0200, Veaceslav Falico wrote:
>> On Tue, Sep 17, 2013 at 02:26:43AM +0100, Ben Hutchings wrote:
>> >On Tue, 2013-09-17 at 02:46 +0200, Veaceslav Falico wrote:
>> >[...]
>> >> --- a/net/core/dev.c
>> >> +++ b/net/core/dev.c
>> >> @@ -4537,6 +4537,72 @@ struct net_device *netdev_all_upper_get_next_dev_rcu(struct net_device *dev,
>> >> }
>> >> EXPORT_SYMBOL(netdev_all_upper_get_next_dev_rcu);
>> >>
>> >> +/* netdev_lower_get_next_private - Get the next ->private from the
>> >> + * lower neighbour list
>> >[...]
>> >
>> >This is not correct kernel-doc syntax. You must begin the comment like
>> >this:
>> >
>> >/**
>> > * function_name - summary on one physical line, no wrapping allowed
>>
>> I've thought that netdev specifically requires that type of comments*. But
>> I don't have any strong opinion on that, so if needed - can change easily
>> in the next version.
>>
>> Thanks a lot!
>>
>> *Documentation/networking/netdev-FAQ.txt
>>
>> Q: Someone said that the comment style and coding convention is different
>> for the networking content. Is this true?
>>
>> A: Yes, in a largely trivial way. Instead of this:
>>
>> /*
>> * foobar blah blah blah
>> * another line of text
>> */
>>
>> it is requested that you make it look like this:
>>
>> /* foobar blah blah blah
>> * another line of text
>> */
>>
>>
>
>Thats for comments, not kernel-doc sections.
>
>You provided :
>
>+/* netdev_lower_get_next_private - Get the next ->private from the
>+ * lower neighbour list
>+ * @dev: device
>+ * @iter: list_head ** of the current position
>+ *
>+ * Gets the next netdev_adjacent->private from the dev's lower neighbour
>+ * list, starting from iter position. The caller must hold either hold the
>+ * RTNL lock or its own locking that guarantees that the neighbour lower
>+ * list will remain unchainged. If iter is NULL - return the first private.
>+ */
>
>Which really looks like a kernel-doc section,
>but misses the proper delimiter which is :
Ok, got it - will fix in the next version.
Thanks all!
>
>/**
>
>Not
>
>/*
>
>
>
^ permalink raw reply
* Re: [PATCH net] tcp: fix RTO calculated from cached RTT
From: Eric Dumazet @ 2013-09-17 12:21 UTC (permalink / raw)
To: Neal Cardwell; +Cc: David Miller, netdev, Eric Dumazet, Yuchung Cheng
In-Reply-To: <1379382260-29303-1-git-send-email-ncardwell@google.com>
On Mon, 2013-09-16 at 21:44 -0400, Neal Cardwell wrote:
> Commit 1b7fdd2ab5852 ("tcp: do not use cached RTT for RTT estimation")
> did not correctly account for the fact that crtt is the RTT shifted
> left 3 bits. Fix the calculation to consistently reflect this fact.
>
> Signed-off-by: Neal Cardwell <ncardwell@google.com>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Yuchung Cheng <ycheng@google.com>
> ---
> net/ipv4/tcp_metrics.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
Acked-by: Eric Dumazet <edumazet@google.com>
Instead of rto = 3*crtt, we had rto = 10*crtt, which was indeed
suboptimal ;)
^ permalink raw reply
* Re: [PATCH net-next] bridge: change the order of actions in addif/delif
From: Eric Dumazet @ 2013-09-17 11:57 UTC (permalink / raw)
To: Hong Zhiguo; +Cc: netdev, stephen, davem, vyasevic, Hong Zhiguo
In-Reply-To: <1379403883-16219-1-git-send-email-zhiguohong@tencent.com>
On Tue, 2013-09-17 at 15:44 +0800, Hong Zhiguo wrote:
> From: Hong Zhiguo <zhiguohong@tencent.com>
>
> netdev_rx_handler_register turns on the bridge traffic. It should
> be the last action of br_add_if, after the installation of fdb and
> stp staff. _Maybe_ traffic coming in before preparation of fdb and
> stp is taken care of, but we could make it easier.
>
> Vise versa, netdev_rx_handler_unregister actually turns off bridge
> traffic from the dev. It should be the first action of br_del_if,
> before fdb and stp staff is uninstalled.
>
> Signed-off-by: Hong Zhiguo <zhiguohong@tencent.com>
> ---
> net/bridge/br_if.c | 20 ++++++++------------
> 1 file changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
> index c41d5fb..6544154 100644
> --- a/net/bridge/br_if.c
> +++ b/net/bridge/br_if.c
> @@ -133,6 +133,8 @@ static void del_nbp(struct net_bridge_port *p)
>
> sysfs_remove_link(br->ifobj, p->dev->name);
>
> + netdev_rx_handler_unregister(dev);
> +
> dev_set_promiscuity(dev, -1);
>
> spin_lock_bh(&br->lock);
> @@ -148,8 +150,6 @@ static void del_nbp(struct net_bridge_port *p)
>
> dev->priv_flags &= ~IFF_BRIDGE_PORT;
>
> - netdev_rx_handler_unregister(dev);
> -
> netdev_upper_dev_unlink(dev, br->dev);
>
This was really your first patch, and I 'Acked' it.
(OK, changelog can be different now we fixed the bug)
The following should be part of a different patch
> br_multicast_del_port(p);
> @@ -336,8 +336,8 @@ int br_add_if(struct net_bridge *br, struct net_device *dev)
> if (dev->netdev_ops->ndo_start_xmit == br_dev_xmit)
> return -ELOOP;
>
> - /* Device is already being bridged */
> - if (br_port_exists(dev))
> + /* Device is already being bridged or registered with other handler */
> + if (br_port_exists(dev) || dev->rx_handler)
> return -EBUSY;
rcu_access_pointer(dev->rx_handler)
Anyway, this is risky, because it assumes netdev_rx_handler_register()
implementation wont change.
Just leave the code as is, I don't think its worth it.
^ permalink raw reply
* Re: [PATCH v3 net-next 07/27] net: add for_each iterators through neighbour lower link's private
From: Eric Dumazet @ 2013-09-17 11:43 UTC (permalink / raw)
To: Veaceslav Falico
Cc: Ben Hutchings, netdev, jiri, David S. Miller, Eric Dumazet,
Alexander Duyck
In-Reply-To: <20130917073618.GC18195@redhat.com>
On Tue, 2013-09-17 at 09:36 +0200, Veaceslav Falico wrote:
> On Tue, Sep 17, 2013 at 02:26:43AM +0100, Ben Hutchings wrote:
> >On Tue, 2013-09-17 at 02:46 +0200, Veaceslav Falico wrote:
> >[...]
> >> --- a/net/core/dev.c
> >> +++ b/net/core/dev.c
> >> @@ -4537,6 +4537,72 @@ struct net_device *netdev_all_upper_get_next_dev_rcu(struct net_device *dev,
> >> }
> >> EXPORT_SYMBOL(netdev_all_upper_get_next_dev_rcu);
> >>
> >> +/* netdev_lower_get_next_private - Get the next ->private from the
> >> + * lower neighbour list
> >[...]
> >
> >This is not correct kernel-doc syntax. You must begin the comment like
> >this:
> >
> >/**
> > * function_name - summary on one physical line, no wrapping allowed
>
> I've thought that netdev specifically requires that type of comments*. But
> I don't have any strong opinion on that, so if needed - can change easily
> in the next version.
>
> Thanks a lot!
>
> *Documentation/networking/netdev-FAQ.txt
>
> Q: Someone said that the comment style and coding convention is different
> for the networking content. Is this true?
>
> A: Yes, in a largely trivial way. Instead of this:
>
> /*
> * foobar blah blah blah
> * another line of text
> */
>
> it is requested that you make it look like this:
>
> /* foobar blah blah blah
> * another line of text
> */
>
>
Thats for comments, not kernel-doc sections.
You provided :
+/* netdev_lower_get_next_private - Get the next ->private from the
+ * lower neighbour list
+ * @dev: device
+ * @iter: list_head ** of the current position
+ *
+ * Gets the next netdev_adjacent->private from the dev's lower neighbour
+ * list, starting from iter position. The caller must hold either hold the
+ * RTNL lock or its own locking that guarantees that the neighbour lower
+ * list will remain unchainged. If iter is NULL - return the first private.
+ */
Which really looks like a kernel-doc section,
but misses the proper delimiter which is :
/**
Not
/*
^ permalink raw reply
* [PATCH iproute2] tc: support TCA_STATS_RATE_EST64
From: Eric Dumazet @ 2013-09-17 11:19 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
From: Eric Dumazet <edumazet@google.com>
Since linux-3.11, rate estimator can provide TCA_STATS_RATE_EST64
when rate (bytes per second) is above 2^32 (~34 Mbits)
Change tc to use this attribute for high rates.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
tc/tc_util.c | 26 ++++++++++++++++++++------
tc/tc_util.h | 4 ++--
2 files changed, 22 insertions(+), 8 deletions(-)
diff --git a/tc/tc_util.c b/tc/tc_util.c
index 8114c97..be3ed07 100644
--- a/tc/tc_util.c
+++ b/tc/tc_util.c
@@ -171,20 +171,24 @@ int get_rate(unsigned *rate, const char *str)
return 0;
}
-void print_rate(char *buf, int len, __u32 rate)
+void print_rate(char *buf, int len, __u64 rate)
{
double tmp = (double)rate*8;
extern int use_iec;
if (use_iec) {
- if (tmp >= 1000.0*1024.0*1024.0)
+ if (tmp >= 1000.0*1024.0*1024.0*1024.0)
+ snprintf(buf, len, "%.0fGibit", tmp/(1024.0*1024.0*1024.0));
+ else if (tmp >= 1000.0*1024.0*1024.0)
snprintf(buf, len, "%.0fMibit", tmp/(1024.0*1024.0));
else if (tmp >= 1000.0*1024)
snprintf(buf, len, "%.0fKibit", tmp/1024);
else
snprintf(buf, len, "%.0fbit", tmp);
} else {
- if (tmp >= 1000.0*1000000.0)
+ if (tmp >= 1000.0*1000000000.0)
+ snprintf(buf, len, "%.0fGbit", tmp/1000000000.0);
+ else if (tmp >= 1000.0*1000000.0)
snprintf(buf, len, "%.0fMbit", tmp/1000000.0);
else if (tmp >= 1000.0 * 1000.0)
snprintf(buf, len, "%.0fKbit", tmp/1000.0);
@@ -193,7 +197,7 @@ void print_rate(char *buf, int len, __u32 rate)
}
}
-char * sprint_rate(__u32 rate, char *buf)
+char * sprint_rate(__u64 rate, char *buf)
{
print_rate(buf, SPRINT_BSIZE-1, rate);
return buf;
@@ -460,9 +464,19 @@ void print_tcstats2_attr(FILE *fp, struct rtattr *rta, char *prefix, struct rtat
q.drops, q.overlimits, q.requeues);
}
- if (tbs[TCA_STATS_RATE_EST]) {
+ if (tbs[TCA_STATS_RATE_EST64]) {
+ struct gnet_stats_rate_est64 re = {0};
+
+ memcpy(&re, RTA_DATA(tbs[TCA_STATS_RATE_EST64]),
+ MIN(RTA_PAYLOAD(tbs[TCA_STATS_RATE_EST64]),
+ sizeof(re)));
+ fprintf(fp, "\n%srate %s %llupps ",
+ prefix, sprint_rate(re.bps, b1), re.pps);
+ } else if (tbs[TCA_STATS_RATE_EST]) {
struct gnet_stats_rate_est re = {0};
- memcpy(&re, RTA_DATA(tbs[TCA_STATS_RATE_EST]), MIN(RTA_PAYLOAD(tbs[TCA_STATS_RATE_EST]), sizeof(re)));
+
+ memcpy(&re, RTA_DATA(tbs[TCA_STATS_RATE_EST]),
+ MIN(RTA_PAYLOAD(tbs[TCA_STATS_RATE_EST]), sizeof(re)));
fprintf(fp, "\n%srate %s %upps ",
prefix, sprint_rate(re.bps, b1), re.pps);
}
diff --git a/tc/tc_util.h b/tc/tc_util.h
index 4f54436..7c3709f 100644
--- a/tc/tc_util.h
+++ b/tc/tc_util.h
@@ -63,12 +63,12 @@ extern int get_size_and_cell(unsigned *size, int *cell_log, char *str);
extern int get_time(unsigned *time, const char *str);
extern int get_linklayer(unsigned *val, const char *arg);
-extern void print_rate(char *buf, int len, __u32 rate);
+extern void print_rate(char *buf, int len, __u64 rate);
extern void print_size(char *buf, int len, __u32 size);
extern void print_qdisc_handle(char *buf, int len, __u32 h);
extern void print_time(char *buf, int len, __u32 time);
extern void print_linklayer(char *buf, int len, unsigned linklayer);
-extern char * sprint_rate(__u32 rate, char *buf);
+extern char * sprint_rate(__u64 rate, char *buf);
extern char * sprint_size(__u32 size, char *buf);
extern char * sprint_qdisc_handle(__u32 h, char *buf);
extern char * sprint_tc_classid(__u32 h, char *buf);
^ permalink raw reply related
* Re: [PATCH net-next] gen_estimator: change the lock order for better perfermance
From: Eric Dumazet @ 2013-09-17 11:11 UTC (permalink / raw)
To: Hong Zhiguo; +Cc: netdev, davem, stephen, Hong Zhiguo
In-Reply-To: <1379407101-18933-1-git-send-email-zhiguohong@tencent.com>
On Tue, 2013-09-17 at 16:38 +0800, Hong Zhiguo wrote:
> From: Hong Zhiguo <zhiguohong@tencent.com>
>
> e->stats_lock is usually taken by fast path to update stats.
> In the old order, fast path should wait for write_lock(&est_lock).
> Even though it's only one line inside the write_lock(&est_lock),
> but if there's interrupt or page fault, a lot of spin on
> e->stats_lock will be wasted in fast path.
1) net-next is not open
2) There is no way a page fault can happen in this path.
3) This patch is wrong.
Current order is there for good reasons.
Have you really tried LOCKDEP, before sending a patch changing lock
order ?
^ permalink raw reply
* Re: [CFT][PATCH] net: Delay default_device_exit_batch until no devices are unregistering
From: Eric W. Biederman @ 2013-09-17 9:38 UTC (permalink / raw)
To: Francesco Ruggeri
Cc: David S. Miller, Eric Dumazet, Jiri Pirko, Alexander Duyck,
Cong Wang, netdev
In-Reply-To: <CA+HUmGjg9jE1fjSMgkfVtiWVRD3d2DuwfbPU3Owrk_cTCe0FQA@mail.gmail.com>
Francesco Ruggeri <fruggeri@aristanetworks.com> writes:
> On Mon, Sep 16, 2013 at 8:49 PM, Eric W. Biederman
> <ebiederm@xmission.com> wrote:
>> Francesco could you take a look at this. I am about 99% certain this is
>> right but I am starting to fade. So it is entirely possible I missed
>> something.
>
> Same here ...
> The logic looks right to me and I think it should address the original
> issue I ran into.
> Would it make sense to have netdev_unregistering and
> netdev_unregistering_wait be per-namespace, and have
> default_device_exit_batch only wait for the namespaces in net_list? It
> would require some extra loops and locking, but it may help avoid
> unnecessary waits.
It might be worth it, and it certainly would give a good progress
guarantee, as activity in an exiting network namespace is guaranteed to
wind down. Progress guarantees are always good to have.
The downside of a per netns unregistering count is that we will have
extra wake ups which will could result in extra contention on the
rtnl_lock.
The wait queue definitely makes sense to be global as there only ever
can be a single waiter, because the netns_wq is single threaded and
everything we are doing is happening with the net_mutex held.
The count doesn't necessarily need to be atomic as it can be protected
by the rtnl_lock.
Like I said the logic is a little rough as I was tired.
I am heading off to New Orleans for Linux Plumbers Conference in a
couple hours so I don't expect I will be particularly responsive
again until next week.
If you could test this patch perhaps refine it I think we are almost at
a final point of fixing this.
Just to be clear my reason for prefering this approach is that because
it adds no extra wait points (we already wait for the rtnl_lock), the
logic is unconditional and explicit and not hidden in the loopback
device's reference count. Which should allow anyone reading the code
to discover and understand this guarantee. Although a big fat comment
in default_device_exit_batch that we are guaranteeing we don't allow
the network namespace to exit while there are still network devices in
it (or something to that effect) is probably appropriate.
Eric
> Francesco
>
>>
>> net/core/dev.c | 12 ++++++++++++
>> 1 files changed, 12 insertions(+), 0 deletions(-)
>>
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 5d702fe..c25e6f3 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -5002,10 +5002,13 @@ static int dev_new_index(struct net *net)
>>
>> /* Delayed registration/unregisteration */
>> static LIST_HEAD(net_todo_list);
>> +static atomic_t netdev_unregistering = ATOMIC_INIT(0);
>> +static DECLARE_WAIT_QUEUE_HEAD(netdev_unregistering_wait);
>>
>> static void net_set_todo(struct net_device *dev)
>> {
>> list_add_tail(&dev->todo_list, &net_todo_list);
>> + atomic_inc(&netdev_unregistering);
>> }
>>
>> static void rollback_registered_many(struct list_head *head)
>> @@ -5673,6 +5676,9 @@ void netdev_run_todo(void)
>> if (dev->destructor)
>> dev->destructor(dev);
>>
>> + if (atomic_dec_and_test(&netdev_unregistering))
>> + wake_up(&netdev_unregistering_wait);
>> +
>> /* Free network device */
>> kobject_put(&dev->dev.kobj);
>> }
>> @@ -6369,7 +6375,13 @@ static void __net_exit default_device_exit_batch(struct list_head *net_list)
>> struct net *net;
>> LIST_HEAD(dev_kill_list);
>>
>> +retry:
>> + wait_event(netdev_unregistering_wait, (atomic_read(&netdev_unregistering) == 0));
>> rtnl_lock();
>> + if (atomic_read(&netdev_unregistering) != 0) {
>> + __rtnl_unlock();
>> + goto retry;
>> + }
>> list_for_each_entry(net, net_list, exit_list) {
>> for_each_netdev_reverse(net, dev) {
>> if (dev->rtnl_link_ops)
>> --
>> 1.7.5.4
>>
^ permalink raw reply
* Re: [PATCH v3 0/6] ipv6: Do route updating for redirect in ndisc layer
From: Daniel Borkmann @ 2013-09-17 9:00 UTC (permalink / raw)
To: Duan Jiong; +Cc: Duan Jiong, davem, netdev, hannes, linux-sctp@vger.kernel.org
In-Reply-To: <523710CC.1090404@gmail.com>
On 09/16/2013 04:08 PM, Duan Jiong wrote:
> 于 2013/9/16 20:22, Daniel Borkmann 写道:
>> On 09/16/2013 01:47 PM, Duan Jiong wrote:
>>> From: Duan Jiong <duanj.fnst@cn.fujitsu.com>
>>>
>>> the ip6_redirect() could be replaced with
>>> ip6_redirect_no_header(), we could always use ip6_redirect()
>>> for route updating in ndisc layer and use the data of the
>>> redirected header option just for finding the socket to be
>>> notified and then notify user in protocols' err_handler.
>> If I get this right, it seems to me that this patchset actually consists of two
>> different kind of changes:
>>
>> 1) Not notifying user space on ICMP redirects (net material)
>> 2) Simplify code for updating route in ndisc layer instead of error handlers (net-next)
>>
>> Also, you do the *actual* change in the very last patch, which means that from
>> patch 1 to 5 we're in an inconsistent and buggy state unless we also apply patch
>> number 6. It should actually be the other way around, that you first do the actual
>> change and then migrate users (also commit messages are quite terse).
>
> I make the patch set on net tree, not on net-next. Maybe those
> things should be done in two patch sets.
>
>> Moreover, just looking at the SCTP part (sctp_err_lookup() function) ...
>>
>> /* RFC 4960, Appendix C. ICMP Handling
>> *
>> * ICMP6) An implementation MUST validate that the Verification Tag
>> * contained in the ICMP message matches the Verification Tag of
>> * the peer. If the Verification Tag is not 0 and does NOT
>> * match, discard the ICMP message. If it is 0 and the ICMP
>> * message contains enough bytes to verify that the chunk type is
>> * an INIT chunk and that the Initiate Tag matches the tag of the
>> * peer, continue with ICMP7. If the ICMP message is too short
>> * or the chunk type or the Initiate Tag does not match, silently
>> * discard the packet.
>> */
>>
>> ... it seems to me that we would simply ignore such RFC requirements with
>> your patch for the sctp_v6_err() part.
>>
>> Care to elaborate? ;-)
>
> Sorry, i didn't notice that.
>
> According to the RFC requirements, it suggests that we can't update
> route for redirect in ndisc layer before calling into sctp_err_lookup(),
> because we must verify the ICMP Message. Now maybe we update route for
> redirect in ndisc layer is wrong.
Looking further into RFC4960 [1] ... it says:
Appendix C. ICMP Handling
Whenever an ICMP message is received by an SCTP endpoint, the
following procedures MUST be followed to ensure proper utilization of
the information being provided by layer 3.
ICMP1) An implementation MAY ignore all ICMPv4 messages where the
type field is not set to "Destination Unreachable".
ICMP2) An implementation MAY ignore all ICMPv6 messages where the
type field is not "Destination Unreachable", "Parameter
Problem", or "Packet Too Big".
...
So this basically means that only packets in step 2 are interesting for us here,
we *may* ignore the rest. The verification comes at step 6, so we may have
ignored the packet first. ;-) Therefore, I think your proposal should not be an
issue for SCTP.
I'd however prefer that your patch handles this similarly as in sctp_v4_err(),
so that in the default switch-case, we just go to out_unlock.
In any case, your commit message would need to be more elaborate and reflect
"why" it is okay/safe to change that in SCTP or other cases, preferably with
reference to the RFC. Otherwise, looking at the Git history in future will just be
confusing as it won't clarify why it was reasonable doing so this way.
[1] http://tools.ietf.org/html/rfc4960#appendix-C
^ permalink raw reply
* [PATCH] USBNET: fix handling padding packet
From: Ming Lei @ 2013-09-17 9:10 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman
Cc: Oliver Neukum, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Ming Lei, Oliver Neukum
Commit 638c5115a7949(USBNET: support DMA SG) introduces DMA SG
if the usb host controller is capable of building packet from
discontinuous buffers, but missed handling padding packet when
building DMA SG.
This patch attachs the pre-allocated padding packet at the
end of the sg list, so padding packet can be sent to device
if drivers require that.
Reported-by: David Laight <David.Laight-JxhZ9S5GRejQT0dZR+AlfA@public.gmane.org>
Cc: Oliver Neukum <oliver-GvhC2dPhHPQdnm+yROfE0A@public.gmane.org>
Signed-off-by: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
drivers/net/usb/usbnet.c | 27 +++++++++++++++++++++------
include/linux/usb/usbnet.h | 1 +
2 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 7b331e6..4a9bed3 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1241,7 +1241,9 @@ static int build_dma_sg(const struct sk_buff *skb, struct urb *urb)
if (num_sgs == 1)
return 0;
- urb->sg = kmalloc(num_sgs * sizeof(struct scatterlist), GFP_ATOMIC);
+ /* reserve one for zero packet */
+ urb->sg = kmalloc((num_sgs + 1) * sizeof(struct scatterlist),
+ GFP_ATOMIC);
if (!urb->sg)
return -ENOMEM;
@@ -1305,7 +1307,7 @@ netdev_tx_t usbnet_start_xmit (struct sk_buff *skb,
if (build_dma_sg(skb, urb) < 0)
goto drop;
}
- entry->length = length = urb->transfer_buffer_length;
+ length = urb->transfer_buffer_length;
/* don't assume the hardware handles USB_ZERO_PACKET
* NOTE: strictly conforming cdc-ether devices should expect
@@ -1317,15 +1319,18 @@ netdev_tx_t usbnet_start_xmit (struct sk_buff *skb,
if (length % dev->maxpacket == 0) {
if (!(info->flags & FLAG_SEND_ZLP)) {
if (!(info->flags & FLAG_MULTI_PACKET)) {
- urb->transfer_buffer_length++;
- if (skb_tailroom(skb)) {
+ length++;
+ if (skb_tailroom(skb) && !dev->can_dma_sg) {
skb->data[skb->len] = 0;
__skb_put(skb, 1);
- }
+ } else if (dev->can_dma_sg)
+ sg_set_buf(&urb->sg[urb->num_sgs++],
+ dev->padding_pkt, 1);
}
} else
urb->transfer_flags |= URB_ZERO_PACKET;
}
+ entry->length = urb->transfer_buffer_length = length;
spin_lock_irqsave(&dev->txq.lock, flags);
retval = usb_autopm_get_interface_async(dev->intf);
@@ -1509,6 +1514,7 @@ void usbnet_disconnect (struct usb_interface *intf)
usb_kill_urb(dev->interrupt);
usb_free_urb(dev->interrupt);
+ kfree(dev->padding_pkt);
free_netdev(net);
}
@@ -1679,9 +1685,16 @@ usbnet_probe (struct usb_interface *udev, const struct usb_device_id *prod)
/* initialize max rx_qlen and tx_qlen */
usbnet_update_max_qlen(dev);
+ if (dev->can_dma_sg && !(info->flags & FLAG_SEND_ZLP) &&
+ !(info->flags & FLAG_MULTI_PACKET)) {
+ dev->padding_pkt = kzalloc(1, GFP_KERNEL);
+ if (!dev->padding_pkt)
+ goto out4;
+ }
+
status = register_netdev (net);
if (status)
- goto out4;
+ goto out5;
netif_info(dev, probe, dev->net,
"register '%s' at usb-%s-%s, %s, %pM\n",
udev->dev.driver->name,
@@ -1699,6 +1712,8 @@ usbnet_probe (struct usb_interface *udev, const struct usb_device_id *prod)
return 0;
+out5:
+ kfree(dev->padding_pkt);
out4:
usb_free_urb(dev->interrupt);
out3:
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index 9cb2fe8..e303eef 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -42,6 +42,7 @@ struct usbnet {
struct usb_host_endpoint *status;
unsigned maxpacket;
struct timer_list delay;
+ const char *padding_pkt;
/* protocol/interface state */
struct net_device *net;
--
1.7.9.5
--
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
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