* 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] 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: 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: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: 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 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: [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
* 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
* [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
* 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
* 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: [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] 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 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
* [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
* [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
* 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 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
* 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
* 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: 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
* [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: [PATCHv3 linux-next] hrtimer: Add notifier when clock_was_set was called
From: Thomas Gleixner @ 2013-09-12 14:43 UTC (permalink / raw)
To: Herbert Xu
Cc: Fan Du, Steffen Klassert, David Miller, Daniel Borkmann, LKML,
netdev
In-Reply-To: <20130912134409.GB21212@gondor.apana.org.au>
On Thu, 12 Sep 2013, Herbert Xu wrote:
> On Thu, Sep 12, 2013 at 03:21:24PM +0200, Thomas Gleixner wrote:
> >
> > > (3): http://www.spinics.net/lists/netdev/msg245169.html
> >
> > Thanks for the explanation so far.
> >
> > What's still unclear to me is why these timeouts are bound to wall
> > time in the first place.
> >
> > Is there any real reason why the key life time can't simply be
> > expressed in monotonic time, e.g. N seconds after creation or M
> > seconds after usage? Looking at the relevant RFCs I can't find any
> > requirement for binding the life time to wall time.
> >
> > A life time of 10 minutes does not change when the wall clock is
> > adjusted for whatever reasons. It's still 10 minutes and not some
> > random result of the wall clock adjustments. But I might be wrong as
> > usual :)
>
> Well we started out with straight timers. It was changed because
> people wanted IPsec SAs to expire after a suspect/resume which
Right suspend is the usual suspect :)
> AFAIK does not touch normal timers.
>
> Of course, this brought with it a new set of problems when the
> system time is stepped which now cause SAs to expire even though
> they probably shouldn't.
Right. That's what I guessed. So your problem is that the timer_list
timers which are the proper mechanism for this (the life time has a 1
second granularity, so hrtimers are complete overkill) are not
expiring after a suspend/resume cycle.
So what about going back to timer_list timers and simply utilize
register_pm_notifier(), which will tell you that the system resumed?
Thanks,
tglx
^ permalink raw reply
* Re: [PATCH net-next] fix NULL pointer dereference in br_handle_frame
From: Vlad Yasevich @ 2013-09-12 14:42 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Hong zhi guo, netdev, David Miller, zhiguohong
In-Reply-To: <1378996396.24408.8.camel@edumazet-glaptop>
On 09/12/2013 10:33 AM, Eric Dumazet wrote:
> On Thu, 2013-09-12 at 22:24 +0800, Hong zhi guo wrote:
>> You mean IFF_BRIDGE_PORT should be only for admin/control path, but
>> not for data path?
>
> By definition, br_handle_frame() should be called only when device is a
> bridge port.
>
> After the call to synchronize_net() included in
> netdev_rx_handler_unregister(), you have guarantee br_handle_frame()
> wont be called.
>
> Therefore, testing IFF_BRIDGE_PORT in br_handle_frame() is redundant.
Don't all tests for IFF_BRIDGE_PORT on the bridge receive path become
redundant as well?
-vlad
>
>
>
> --
> 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
* Twice the action, double the fun!
From: ROBERTLBEMIS @ 2013-09-12 14:35 UTC (permalink / raw)
Dear Customer
Make a first deposit to receive your first casino bonus.
http://onlinecasino46.webnode.com/casino/casino3
Best Regards,
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox