* Re: [PATCHv3 net-next-2.6 2/3] qlcnic: Take FW dump via ethtool
From: Anirban Chakraborty @ 2011-05-12 20:54 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev, David Miller
In-Reply-To: <1305227938.5214.48.camel@bwh-desktop>
On May 12, 2011, at 12:18 PM, Ben Hutchings wrote:
> On Thu, 2011-05-12 at 11:53 -0700, Anirban Chakraborty wrote:
>> On May 12, 2011, at 10:27 AM, Ben Hutchings wrote:
>>
>>> On Wed, 2011-05-11 at 15:54 -0700, Anirban Chakraborty wrote:
>>>> Driver checks if the previous dump has been cleared before taking the dump.
>>>> It doesn't take the dump if it is not cleared.
>>>>
>>>> Changes from v2:
>>>> Added lock to protect dump data structures from being mangled while
>>>> dumping or setting them via ethtool.
>>>
>>> Unfortunately it still seems to be possible for the dump length to
>>> change between the ethtool core calling qlcnic_get_dump_flag() and
>>> qlcnic_get_dump_data().
>>
>> dump length is serialized via the driver lock. dump length is a static
>> entity for a given capture mask and it can only be changed when there
>> is a different capture mask set in the driver (via calling set_dump()
>> from ethtool core).
>
> OK.
>
>> Actual dump size is determined during the initial steps of FW dump
>> which takes the driver lock to start with. So, I am not sure how the
>> dump length could be changed between the calls to get_dump_flag and
>> get_dump_data from within the ethtool core without a call to
>> set_dump() in between.
> [...]
>
> What prevents this sequence:
>
> 1. Driver detects firmware dump is required, and creates the dump
> (length L1).
> 2. User changes firmware dump flags through ethtool.
> 3. User starts to save firmware dump through ethtool:
> a. ethtool utility reads dump length (= L1) and allocates user buffer
> b. ethtool utility reads dump:
> c. ethtool core reads dump length (L1) and allocates kernel buffer
> 4. Meanwhile, driver detects firmware dump is required again, and
> creates a new dump (length L2, since dump flags changed)
This step won't happen as driver ensures that if a dump is taken that is not
cleared yet by ethtool utility, it is not going to take any further dump. So, until
the get_dump_data() has been called to fetch the dump data, changing dump
flag (and hence dump size) will not have any effect.
In 3 above, ethtool core created a buffer of L1 size but hasn't yet called
get_dump_data() of the driver, so driver is still holding onto its previously
dumped data even though capture mask has been changed and the driver
encountered a situation where it ought to take the dump.
-Anirban
^ permalink raw reply
* Re: [PATCHv3 net-next-2.6 2/3] qlcnic: Take FW dump via ethtool
From: Ben Hutchings @ 2011-05-12 21:02 UTC (permalink / raw)
To: Anirban Chakraborty; +Cc: netdev, David Miller
In-Reply-To: <135A6631-C557-4C01-9FDD-F9E58F555168@qlogic.com>
On Thu, 2011-05-12 at 13:54 -0700, Anirban Chakraborty wrote:
> On May 12, 2011, at 12:18 PM, Ben Hutchings wrote:
[...]
> > What prevents this sequence:
> >
> > 1. Driver detects firmware dump is required, and creates the dump
> > (length L1).
> > 2. User changes firmware dump flags through ethtool.
> > 3. User starts to save firmware dump through ethtool:
> > a. ethtool utility reads dump length (= L1) and allocates user buffer
> > b. ethtool utility reads dump:
> > c. ethtool core reads dump length (L1) and allocates kernel buffer
> > 4. Meanwhile, driver detects firmware dump is required again, and
> > creates a new dump (length L2, since dump flags changed)
>
> This step won't happen as driver ensures that if a dump is taken that is not
> cleared yet by ethtool utility, it is not going to take any further dump. So, until
> the get_dump_data() has been called to fetch the dump data, changing dump
> flag (and hence dump size) will not have any effect.
> In 3 above, ethtool core created a buffer of L1 size but hasn't yet called
> get_dump_data() of the driver, so driver is still holding onto its previously
> dumped data even though capture mask has been changed and the driver
> encountered a situation where it ought to take the dump.
OK, that sounds safe.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH 0/3] Fix 8390 regressions
From: David Miller @ 2011-05-12 21:02 UTC (permalink / raw)
To: geert; +Cc: shemminger, ysato, linux, fthain, netdev, linux-kernel,
linux-m68k
In-Reply-To: <1305227500-15595-1-git-send-email-geert@linux-m68k.org>
From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: Thu, 12 May 2011 19:11:37 +0000
> These patches fix regressions in 3 8390-based network drivers:
> [1/3] zorro8390: Fix regression caused during net_device_ops conversion
> [2/3] hydra: Fix regression caused during net_device_ops conversion
> [3/3] ne-h8300: Fix regression caused during net_device_ops conversion
>
> The first one, for zorro8390, has been tested.
> The second one, for hydra, has been compile-tested only.
>
> Based on commits 8cfd9e923be54ef66ce174a93f4592b444b96407 ("[ARM] RiscPC: Fix
> etherh oops") and 217cbfa856dc1cbc2890781626c4032d9e3ec59f ("mac8390: fix
> regression caused during net_device_ops conversion"), and the patch for
> zorro8390, we have good reasons to believe hydra and ne-h8300 are affected
> as well, as they all include lib8390.c.
> Hence patches for those are also included, although I could not test them.
All applied, thanks Geert, and I made the "From: " removals you requested.
^ permalink raw reply
* Re: [PATCH 1/2] sctp: sctp_sendmsg: Don't initialize default_sinfo
From: David Miller @ 2011-05-12 21:06 UTC (permalink / raw)
To: joe; +Cc: vladislav.yasevich, sri, linux-sctp, netdev, linux-kernel
In-Reply-To: <a4e846ed62d7f9222dc9b09ca00b5356a1eb9b73.1305227620.git.joe@perches.com>
From: Joe Perches <joe@perches.com>
Date: Thu, 12 May 2011 12:19:09 -0700
> This variable only needs initialization when cmsgs.info
> is NULL.
>
> Don't use memset, just initialize every struct member.
>
> Signed-off-by: Joe Perches <joe@perches.com>
I don't think you do this, this structure has padding holes on pretty
much every architecture.
It starts with 3 u16's, then there is a u32, so there is a 2-byte
piece of padding after the 3rd u16.
Can you prove that these uninitialized portions never make it to
userspace? If you can, that proof belongs in the commit message.
I think it's too risky.
^ permalink raw reply
* Re: Kernel crash after using new Intel NIC (igb)
From: Arun Sharma @ 2011-05-12 21:10 UTC (permalink / raw)
To: Maximilian Engelhardt
Cc: Eric Dumazet, linux-kernel, netdev, StuStaNet Vorstand
In-Reply-To: <201104271352.00601.maxi@daemonizer.de>
On Wed, Apr 27, 2011 at 01:51:59PM +0200, Maximilian Engelhardt wrote:
>
> thank you for this information. I updated the kernel of the affected server to
> version 2.6.38.4 yesterday. I'll report when there are still crashes, but it
> might take a while, as in the past they only happened within the interval of
> weeks to month.
I've seen this panic on 2.6.38.y as well. It's rare (< 1%), but the
panic consistently happens at the same place.
Are any of the inetpeer.c commits in the 2.6.39 tree, which are not in -stable yet,
good candidates to try?
-Arun
^ permalink raw reply
* Re: Kernel crash after using new Intel NIC (igb)
From: Eric Dumazet @ 2011-05-12 21:15 UTC (permalink / raw)
To: Arun Sharma
Cc: Maximilian Engelhardt, linux-kernel, netdev, StuStaNet Vorstand
In-Reply-To: <20110512211033.GA3468@dev1756.snc6.facebook.com>
Le jeudi 12 mai 2011 à 14:10 -0700, Arun Sharma a écrit :
> On Wed, Apr 27, 2011 at 01:51:59PM +0200, Maximilian Engelhardt wrote:
> >
> > thank you for this information. I updated the kernel of the affected server to
> > version 2.6.38.4 yesterday. I'll report when there are still crashes, but it
> > might take a while, as in the past they only happened within the interval of
> > weeks to month.
>
> I've seen this panic on 2.6.38.y as well. It's rare (< 1%), but the
> panic consistently happens at the same place.
>
> Are any of the inetpeer.c commits in the 2.6.39 tree, which are not in -stable yet,
> good candidates to try?
>
Probably not.
What gives slub_nomerge=1 for you ?
^ permalink raw reply
* Re: [PATCH] net: af_packet: Don't initialize vnet_hdr
From: David Miller @ 2011-05-12 21:26 UTC (permalink / raw)
To: joe; +Cc: linux-kernel, netdev
In-Reply-To: <18a010357eb63dfd50751c5eb529f6261cf90ecd.1305232529.git.joe@perches.com>
From: Joe Perches <joe@perches.com>
Date: Thu, 12 May 2011 13:36:10 -0700
> Save a memset, initialize only the portion necessary.
>
> packet_snd either gets this structure completely from
> memcpy_fromiovec or uses only the hdr_len set to 0,
> so don't always initialize the structure to 0.
>
> Signed-off-by: Joe Perches <joe@perches.com>
On ARM this won't be tightly packed, therefore you'll leave
uninitialized pieces of padding in this structure and this
thing is an "on-the-wire" network header.
I'm not applying this.
^ permalink raw reply
* [PATCH 1/2 v2] sctp: sctp_sendmsg: Don't initialize default_sinfo
From: Joe Perches @ 2011-05-12 21:27 UTC (permalink / raw)
To: David Miller; +Cc: vladislav.yasevich, sri, linux-sctp, netdev, linux-kernel
In-Reply-To: <20110512.170603.549349521517995900.davem@davemloft.net>
This variable only needs initialization when cmsgs.info
is NULL.
Use memset to ensure padding is also zeroed so
kernel doesn't leak any data.
Signed-off-by: Joe Perches <joe@perches.com>
---
On Thu, 2011-05-12 at 17:06 -0400, David Miller wrote:
From: Joe Perches <joe@perches.com>
> Date: Thu, 12 May 2011 12:19:09 -0700
> > This variable only needs initialization when cmsgs.info
> > is NULL.
> > Don't use memset, just initialize every struct member.
> > Signed-off-by: Joe Perches <joe@perches.com>
> I don't think you do this, this structure has padding holes on pretty
> much every architecture.
> It starts with 3 u16's, then there is a u32, so there is a 2-byte
> piece of padding after the 3rd u16.
> Can you prove that these uninitialized portions never make it to
> userspace? If you can, that proof belongs in the commit message.
Thanks David. I didn't notice it went to userspace.
> I think it's too risky.
It is. I like memset.
The current initialization isn't guaranteed by c90 standard
to zero all padding either. In practice it does though.
The idea was to avoid doing a (non-memset) struct foo bar = {}
when unnecessary for every packet as it's only needed when
cmsgs.info is NULL.
net/sctp/socket.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 33d9ee6..d4b8db1 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -1496,7 +1496,7 @@ SCTP_STATIC int sctp_sendmsg(struct kiocb *iocb, struct sock *sk,
struct sctp_chunk *chunk;
union sctp_addr to;
struct sockaddr *msg_name = NULL;
- struct sctp_sndrcvinfo default_sinfo = { 0 };
+ struct sctp_sndrcvinfo default_sinfo;
struct sctp_sndrcvinfo *sinfo;
struct sctp_initmsg *sinit;
sctp_assoc_t associd = 0;
@@ -1760,6 +1760,7 @@ SCTP_STATIC int sctp_sendmsg(struct kiocb *iocb, struct sock *sk,
/* If the user didn't specify SNDRCVINFO, make up one with
* some defaults.
*/
+ memset(&default_sinfo, 0, sizeof(default_sinfo));
default_sinfo.sinfo_stream = asoc->default_stream;
default_sinfo.sinfo_flags = asoc->default_flags;
default_sinfo.sinfo_ppid = asoc->default_ppid;
^ permalink raw reply related
* Re: [PATCH net-next-2.6] l2tp: fix potential rcu race
From: David Miller @ 2011-05-12 21:27 UTC (permalink / raw)
To: paulmck; +Cc: eric.dumazet, netdev, jchapman
In-Reply-To: <20110512142641.GO2258@linux.vnet.ibm.com>
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Date: Thu, 12 May 2011 07:26:41 -0700
> On Thu, May 12, 2011 at 06:22:36AM +0200, Eric Dumazet wrote:
>> While trying to remove useless synchronize_rcu() calls, I found l2tp is
>> indeed incorrectly using two of such calls, but also bumps tunnel
>> refcount after list insertion.
>>
>> tunnel refcount must be incremented before being made publically visible
>> by rcu readers.
>>
>> This fix can be applied to 2.6.35+ and might need a backport for older
>> kernels, since things were shuffled in commit fd558d186df2c
>> (l2tp: Split pppol2tp patch into separate l2tp and ppp parts)
>
> Ouch! Good catch, Eric!
>
> Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Applied, thanks Eric.
^ permalink raw reply
* Re: [PATCH net-next-2.6 2/2] be2net: fix mbox polling for signal reception
From: David Miller @ 2011-05-12 21:29 UTC (permalink / raw)
To: bhutchings; +Cc: sathya.perla, netdev
In-Reply-To: <1305204546.4065.395.camel@localhost>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Thu, 12 May 2011 13:49:06 +0100
> On Thu, 2011-05-12 at 11:41 +0530, Sathya Perla wrote:
>> Sending mbox cmds require multiple steps of writing to the DB register and polling
>> for an ack. Gettting interrupted in the middle by a signal breaks the mbox protocol.
>> So, set the task to UNINTERRUPTIBLE for mbox polling.
>>
>> Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
>> ---
>> drivers/net/benet/be_cmds.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/net/benet/be_cmds.c b/drivers/net/benet/be_cmds.c
>> index bff41ed..55c8301 100644
>> --- a/drivers/net/benet/be_cmds.c
>> +++ b/drivers/net/benet/be_cmds.c
>> @@ -297,7 +297,7 @@ static int be_mbox_db_ready_wait(struct be_adapter *adapter, void __iomem *db)
>> return -1;
>> }
>>
>> - set_current_state(TASK_INTERRUPTIBLE);
>> + set_current_state(TASK_UNINTERRUPTIBLE);
>> schedule_timeout(msecs_to_jiffies(1));
>
> msleep(1) is a lot more readable.
Agreed.
Sathya, please make this change and resubmit this patch series.
Thanks.
^ permalink raw reply
* Re: [PATCH 1/2 v2] sctp: sctp_sendmsg: Don't initialize default_sinfo
From: David Miller @ 2011-05-12 21:31 UTC (permalink / raw)
To: joe; +Cc: vladislav.yasevich, sri, linux-sctp, netdev, linux-kernel
In-Reply-To: <1305235641.6124.64.camel@Joe-Laptop>
From: Joe Perches <joe@perches.com>
Date: Thu, 12 May 2011 14:27:20 -0700
> This variable only needs initialization when cmsgs.info
> is NULL.
>
> Use memset to ensure padding is also zeroed so
> kernel doesn't leak any data.
>
> Signed-off-by: Joe Perches <joe@perches.com>
This looks a lot better, applied.
^ permalink raw reply
* Re: [PATCH 2/2] sctp: sctp_sendmsg: Don't test known non-null sinfo
From: David Miller @ 2011-05-12 21:31 UTC (permalink / raw)
To: joe; +Cc: vladislav.yasevich, sri, linux-sctp, netdev, linux-kernel
In-Reply-To: <b6372a0ac574e3fd110de57eb3e41d97c73d5423.1305227621.git.joe@perches.com>
From: Joe Perches <joe@perches.com>
Date: Thu, 12 May 2011 12:19:10 -0700
> It's already known non-null above.
>
> Signed-off-by: Joe Perches <joe@perches.com>
Applied.
^ permalink raw reply
* Re: [PATCH] net: af_packet: Don't initialize vnet_hdr
From: Joe Perches @ 2011-05-12 21:33 UTC (permalink / raw)
To: David Miller; +Cc: linux-kernel, netdev
In-Reply-To: <20110512.172623.220889608152651989.davem@davemloft.net>
On Thu, 2011-05-12 at 17:26 -0400, David Miller wrote:
> From: Joe Perches <joe@perches.com>
> Date: Thu, 12 May 2011 13:36:10 -0700
>
> > Save a memset, initialize only the portion necessary.
> >
> > packet_snd either gets this structure completely from
> > memcpy_fromiovec or uses only the hdr_len set to 0,
> > so don't always initialize the structure to 0.
> >
> > Signed-off-by: Joe Perches <joe@perches.com>
>
> On ARM this won't be tightly packed, therefore you'll leave
> uninitialized pieces of padding in this structure and this
> thing is an "on-the-wire" network header.
>
> I'm not applying this.
I believe that it's only sent when po->has_vnet_hdr
is set. In that case, it's completely filled from
memcpy_fromiovec. In the not set po->has_vnet_hdr case,
only vnet_hdr.hdr_len is accessed by packet_alloc_skb.
cheers, Joe
^ permalink raw reply
* Re: [PATCH] net: af_packet: Don't initialize vnet_hdr
From: David Miller @ 2011-05-12 21:36 UTC (permalink / raw)
To: joe; +Cc: linux-kernel, netdev
In-Reply-To: <1305236000.6124.67.camel@Joe-Laptop>
From: Joe Perches <joe@perches.com>
Date: Thu, 12 May 2011 14:33:20 -0700
> On Thu, 2011-05-12 at 17:26 -0400, David Miller wrote:
>> From: Joe Perches <joe@perches.com>
>> Date: Thu, 12 May 2011 13:36:10 -0700
>>
>> > Save a memset, initialize only the portion necessary.
>> >
>> > packet_snd either gets this structure completely from
>> > memcpy_fromiovec or uses only the hdr_len set to 0,
>> > so don't always initialize the structure to 0.
>> >
>> > Signed-off-by: Joe Perches <joe@perches.com>
>>
>> On ARM this won't be tightly packed, therefore you'll leave
>> uninitialized pieces of padding in this structure and this
>> thing is an "on-the-wire" network header.
>>
>> I'm not applying this.
>
> I believe that it's only sent when po->has_vnet_hdr
> is set. In that case, it's completely filled from
> memcpy_fromiovec. In the not set po->has_vnet_hdr case,
> only vnet_hdr.hdr_len is accessed by packet_alloc_skb.
I think with the way this code is protecting accesses to vnet_hdr,
it's going to start warning that some parts "may" be use
uninitialized.
It's also slightly ugly to partially initialize these on-stack things.
I would rather see the code rearranged such that this sort of
hackish scheme isn't necessary.
Again, I'm not applying this.
^ permalink raw reply
* Re: [PATCH net-next] net:set valid name before calling ndo_init()
From: David Miller @ 2011-05-12 21:37 UTC (permalink / raw)
To: eric.dumazet
Cc: panweiping3, therbert, mirq-linux, bhutchings, netdev,
linux-kernel
In-Reply-To: <1305214382.3795.28.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 12 May 2011 17:33:02 +0200
> Le jeudi 12 mai 2011 à 21:39 +0800, Weiping Pan(潘卫平) a écrit :
>> From: Pan Weiping <panweiping3@gmail.com>
>>
>> A bug of bonding was invloved by e815d19ffe02bdfda1260949ef2b1806171,
>> see example 1 and 2.
>>
>
> I cant find e815d19ffe02bdfda1260949ef2b1806171 in net-next-2.6
>
> but I do find 1c5cae815d19ffe02bdfda1260949ef2b1806171
>
> Please always use following when referring to a commit :
>
> ... in commit 1c5cae815d19 (net: call dev_alloc_name from
> register_netdevice) ...
>
> - just limit to _first_ 12 chars, no need to have full length
> - give the commit title
Pan, please fix this commit reference up and resubmit your patch.
^ permalink raw reply
* Re: [PATCH net-next] vmxnet3: Use single tx queue when CONFIG_PCI_MSI not defined
From: David Miller @ 2011-05-12 21:37 UTC (permalink / raw)
To: sbhatewara; +Cc: netdev, linux-kernel, pv-drivers
In-Reply-To: <alpine.LRH.2.00.1105100903110.1095@sbhatewara-dev1.eng.vmware.com>
From: Shreyas Bhatewara <sbhatewara@vmware.com>
Date: Tue, 10 May 2011 09:13:56 -0700 (PDT)
>
> Resending this patch with few changes.
>
>
> Avoid multiple queues when MSI or MSI-X not available
>
> Limit number of Tx queues to 1 if MSI/MSI-X support is not configured in
> the kernel. This will make number of tx and rx queues equal when MSI/X
> is not configured thus providing better performance.
>
> Signed-off-by: Bhavesh Davda <bhavesh@vmware.com>
> Signed-off-by: Shreyas N Bhatewara <sbhatewara@vmware.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCHv1] e1000e: Allow ethtool to enable/disable loopback.
From: Mahesh Bandewar @ 2011-05-12 21:41 UTC (permalink / raw)
To: Michał Mirosław; +Cc: e1000-devel, netdev, Tom Herbert, David Miller
In-Reply-To: <BANLkTi=YbTgy0cNJRvcg1QkWCv9sNKkvUQ@mail.gmail.com>
On Wed, May 11, 2011 at 10:50 PM, Michał Mirosław <mirqus@gmail.com> wrote:
> W dniu 12 maja 2011 01:11 użytkownik Mahesh Bandewar
> <maheshb@google.com> napisał:
>> On Wed, May 11, 2011 at 12:15 PM, Michał Mirosław <mirqus@gmail.com> wrote:
>>> 2011/5/11 Mahesh Bandewar <maheshb@google.com>:
>>>> This patch adds e1000_set_features() to handle loopback mode. When loopback
>>>> is enabled, it enables internal-MAC loopback.
>>> Please wait for this driver's conversion to hw_features. One comment
>>> below, though.
>> This is not intrusive so should not create problems when that happens.
>
> Fine by me then.
>
>>> [...]
>>>> --- a/drivers/net/e1000e/netdev.c
>>>> +++ b/drivers/net/e1000e/netdev.c
>>> [...]
>>>> +static int e1000_set_features(struct net_device *dev, u32 features)
>>>> +{
>>>> + u32 changed = dev->features ^ features;
>>>> +
>>>> + if ((changed & NETIF_F_LOOPBACK) && netif_running(dev))
>>>> + e1000_set_loopback(dev, features);
>>>> +
>>>> + return 0;
>>>> +}
>>> [...]
>>>
>>> If e1000_set_loopback() fails, this should set dev->features to passed
>>> features (but keeping NETIF_F_LOOPBACK unchanged in dev->features) to
>>> keep the state consistent.
>> set_features() can return the return code of set_loopback() instead of
>> 0; this way the consistency will be maintained.
>
> Only as long as NETIF_F_LOOPBACK is the only bit set in hw_features.
> netdev_update_features() can't really know which features were changed
> and which failed when ndo_set_features callback returns non-zero.
>
This is more of an API shortcoming. Callback will have to revert
changes made (rollback) before returning non-zero value to keep it
consistent.
Thanks,
--mahesh..
> Best Regards,
> Michał Mirosław
>
------------------------------------------------------------------------------
Achieve unprecedented app performance and reliability
What every C/C++ and Fortran developer should know.
Learn how Intel has extended the reach of its next-generation tools
to help boost performance applications - inlcuding clusters.
http://p.sf.net/sfu/intel-dev2devmay
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply
* Re: [PATCH net-next-2.6] garp: remove last synchronize_rcu() call
From: David Miller @ 2011-05-12 21:47 UTC (permalink / raw)
To: eric.dumazet; +Cc: kaber, greearb, netdev, paulmck
In-Reply-To: <1305206957.3795.21.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 12 May 2011 15:29:17 +0200
> [PATCH net-next-2.6] garp: remove last synchronize_rcu() call
>
> When removing last vlan from a device, garp_uninit_applicant() calls
> synchronize_rcu() to make sure no user can still manipulate struct
> garp_applicant before we free it.
>
> Use call_rcu() instead, as a step to further net_device dismantle
> optimizations.
>
> Add the temporary garp_cleanup_module() function to make sure no pending
> call_rcu() are left at module unload time [ this will be removed when
> kfree_rcu() is available ]
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Looks good, applied, thanks Eric.
^ permalink raw reply
* Re: [PATCH RESEND net-next] ethtool: bring back missing comma in netdev features strings
From: David Miller @ 2011-05-12 21:50 UTC (permalink / raw)
To: mirqus; +Cc: franco, netdev, maheshb
In-Reply-To: <BANLkTikW=vURi7sAQ9ZJdgy=JDv+JacTQQ@mail.gmail.com>
From: Michał Mirosław <mirqus@gmail.com>
Date: Thu, 12 May 2011 18:45:12 +0200
> 2011/5/12 <franco@lastsummer.de>:
>> The issue was introduced in commit eed2a12f1ed9aabf.
>>
>> Signed-off-by: Franco Fichtner <franco@lastsummer.de>
>> ---
>> net/core/ethtool.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
>> index b6f4058..b8c2b10 100644
>> --- a/net/core/ethtool.c
>> +++ b/net/core/ethtool.c
>> @@ -361,7 +361,7 @@ static const char
>> netdev_features_strings[ETHTOOL_DEV_FEATURE_WORDS * 32][ETH_GS
>> /* NETIF_F_NTUPLE */ "rx-ntuple-filter",
>> /* NETIF_F_RXHASH */ "rx-hashing",
>> /* NETIF_F_RXCSUM */ "rx-checksum",
>> - /* NETIF_F_NOCACHE_COPY */ "tx-nocache-copy"
>> + /* NETIF_F_NOCACHE_COPY */ "tx-nocache-copy",
>> /* NETIF_F_LOOPBACK */ "loopback",
>> };
>
> Acked-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Applied, but this patch was significantly mangled by your email
client. Please correct this problem before submitting future
patches, as I will just push them back to you if this happens
again.
Thanks.
^ permalink raw reply
* Re: [PATCH] net: af_packet: Don't initialize vnet_hdr
From: Joe Perches @ 2011-05-12 21:55 UTC (permalink / raw)
To: David Miller; +Cc: linux-kernel, netdev
In-Reply-To: <20110512.173608.1652572492952866283.davem@davemloft.net>
Save an initialization because when this structure
is used it's completely filled by memcpy_fromiovec.
Add a new variable used for the 0 sized allocation
when this structure is not used.
Signed-off-by: Joe Perches <joe@perches.com>
---
On Thu, 2011-05-12 at 17:36 -0400, David Miller wrote:
> I would rather see the code rearranged such that this sort of
> hackish scheme isn't necessary.
net/packet/af_packet.c | 10 +++++++---
1 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 549527b..cc1e83d 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1123,9 +1123,10 @@ static int packet_snd(struct socket *sock,
__be16 proto;
unsigned char *addr;
int ifindex, err, reserve = 0;
- struct virtio_net_hdr vnet_hdr = { 0 };
+ struct virtio_net_hdr vnet_hdr;
int offset = 0;
int vnet_hdr_len;
+ size_t alloc_vnet_hdr_len;
struct packet_sock *po = pkt_sk(sk);
unsigned short gso_type = 0;
@@ -1206,7 +1207,10 @@ static int packet_snd(struct socket *sock,
goto out_unlock;
}
- }
+ alloc_vnet_hdr_len = vnet_hdr.hdr_len;
+ } else
+ alloc_vnet_hdr_len = 0;
+
err = -EMSGSIZE;
if (!gso_type && (len > dev->mtu + reserve + VLAN_HLEN))
@@ -1214,7 +1218,7 @@ static int packet_snd(struct socket *sock,
err = -ENOBUFS;
skb = packet_alloc_skb(sk, LL_ALLOCATED_SPACE(dev),
- LL_RESERVED_SPACE(dev), len, vnet_hdr.hdr_len,
+ LL_RESERVED_SPACE(dev), len, alloc_vnet_hdr_len,
msg->msg_flags & MSG_DONTWAIT, &err);
if (skb == NULL)
goto out_unlock;
^ permalink raw reply related
* Re: [PATCH] net: Fix vlan_features propagation
From: David Miller @ 2011-05-12 21:55 UTC (permalink / raw)
To: mirq-linux; +Cc: netdev, kaber
In-Reply-To: <20110506175629.AF59713A6A@rere.qmqm.pl>
From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Date: Fri, 6 May 2011 19:56:29 +0200 (CEST)
> Fix VLAN features propagation for devices which change vlan_features.
> For this to work, driver needs to make sure netdev_features_changed()
> gets called after the change (it is e.g. after ndo_set_features()).
>
> Side effect is that a user might request features that will never
> be enabled on a VLAN device.
>
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Applied.
^ permalink raw reply
* Re: [PATCH v2] net: group FCoE related feature flags
From: David Miller @ 2011-05-12 21:55 UTC (permalink / raw)
To: yi.zou; +Cc: netdev, mirq-linux, jeffrey.t.kirsher, devel
In-Reply-To: <20110509215133.4346.14504.stgit@localhost6.localdomain6>
From: Yi Zou <yi.zou@intel.com>
Date: Mon, 09 May 2011 14:53:27 -0700
> Michał Mirosław's patch (http://patchwork.ozlabs.org/patch/94421/) fixes the
> issue (http://patchwork.ozlabs.org/patch/94188/) about not populating FCoE related
> flags correctly on vlan devices. However, only NETIF_F_FCOE_CRC is part of the
> NETIF_F_ALL_TX_OFFLOADS right now, where weed NETIF_F_FCOE_MTU and NETIF_F_FSO
> as well.
>
> Therefore, add NETIF_F_ALL_FCOE to indicate feature flags used by FCoE TX offloads.
> These include NETIF_F_FCOE_CRC, NETIF_F_FCOE_MTU, and NETIF_F_FSO and add them to
> be part of NETIF_F_ALL_TX_OFFLOADS. This would eventually make sure all FCoE needed
> flags are populated properly to vlan devices.
>
> Signed-off-by: Yi Zou <yi.zou@intel.com>
Applied.
^ permalink raw reply
* Re: [PATCH] net: af_packet: Don't initialize vnet_hdr
From: David Miller @ 2011-05-12 21:59 UTC (permalink / raw)
To: joe; +Cc: linux-kernel, netdev
In-Reply-To: <1305237348.6124.72.camel@Joe-Laptop>
From: Joe Perches <joe@perches.com>
Date: Thu, 12 May 2011 14:55:48 -0700
> Save an initialization because when this structure
> is used it's completely filled by memcpy_fromiovec.
>
> Add a new variable used for the 0 sized allocation
> when this structure is not used.
>
> Signed-off-by: Joe Perches <joe@perches.com>
>
> ---
>
> On Thu, 2011-05-12 at 17:36 -0400, David Miller wrote:
>> I would rather see the code rearranged such that this sort of
>> hackish scheme isn't necessary.
You misunderstood me.
It's this:
struct virtio_net_hdr vnet_hdr;
if (po->has_vnet_hdr) {
initialize &vnet_hdr
}
...
if (po->has_vnet_hdr) {
use vnet_hdr
}
which I'm talking about when I say "hackish scheme".
The compiler cannot conclusively see that the control flow always
goes to the code that initialized vnet_hdr every time it reaches
the code that uses it.
I want _that_ part rearranged, not what you decided to tackle
here.
For the third time, I'm not applying your patch.
^ permalink raw reply
* Re: [patch 1/9] [PATCH] qeth: convert to hw_features part 2
From: David Miller @ 2011-05-12 22:02 UTC (permalink / raw)
To: blaschka; +Cc: mirqus, netdev, linux-s390
In-Reply-To: <20110512115959.GA37368@tuxmaker.boeblingen.de.ibm.com>
From: Frank Blaschka <blaschka@linux.vnet.ibm.com>
Date: Thu, 12 May 2011 13:59:59 +0200
> Since this is RX processing there is nothing queued during recovery. But
> you are right for tx csum or TSO we have to think about this. For now
> I would like to complete this patch.
Can you guys resolve this quickly so that I can apply this patch
series?
^ permalink raw reply
* Re: [PATCH] net/irda/ircomm_tty.c: Use flip buffers to deliver data
From: David Miller @ 2011-05-12 22:03 UTC (permalink / raw)
To: alan
Cc: amit.virdi, samuel, alan, eric.dumazet, netdev, linux-kernel,
shiraz.hashim, armando.visconti, viresh.kumar
In-Reply-To: <20110512121751.61d62446@lxorguk.ukuu.org.uk>
From: Alan Cox <alan@lxorguk.ukuu.org.uk>
Date: Thu, 12 May 2011 12:17:51 +0100
> On Thu, 12 May 2011 16:34:40 +0530
> Amit Virdi <amit.virdi@st.com> wrote:
>
>> use tty_insert_flip_string and tty_flip_buffer_push to deliver incoming data
>> packets from the IrDA device instead of delivering the packets directly to the
>> line discipline. Following later approach resulted in warning "Sleeping function
>> called from invalid context".
>>
>> Signed-off-by: Amit Virdi <amit.virdi@st.com>
>
> Acked-by: Alan Cox <alan@linux.intel.com>
Applied to net-next-2.6, thanks.
^ 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