* Re: tg3 driver upgrade (Linux 2.6.32 -> 3.2) breaks IBM Bladecenter SoL
From: Ben Hutchings @ 2012-10-03 0:17 UTC (permalink / raw)
To: Michael Tokarev, David Miller
Cc: Ferenc Wagner, Michael Chan, netdev, Matt Carlson, Grant Likely,
Rob Herring, linux-kernel, stable
In-Reply-To: <506B3B52.7080001@msgid.tls.msk.ru>
[-- Attachment #1: Type: text/plain, Size: 1319 bytes --]
On Tue, 2012-10-02 at 23:06 +0400, Michael Tokarev wrote:
> On 02.10.2012 22:49, Ferenc Wagner wrote:
> > "Michael Chan" <mchan@broadcom.com> writes:
> >> These are the likely fixes:
> >>
> >> commit cf9ecf4b631f649a964fa611f1a5e8874f2a76db
> >> Author: Matt Carlson <mcarlson@broadcom.com>
> >> Date: Mon Nov 28 09:41:03 2011 +0000
> >>
> >> tg3: Fix TSO CAP for 5704 devs w / ASF enabled
> >
> > You are exactly right: cf9ecf4b fixed the premanent SoL breakage
> > introduced by dabc5c67. Looks like ASF utilizes similar technology to
> > that of the HS20 BMC. Thanks for the tip, it greatly reduced our CPU
> > wear. :) It's a pity ethtool -k did not give a hint. Do you think it's
> > possible to work around in 3.2 by eg. fiddling some ethtool setting?
>
> Maybe it's better to push this commit to -stable instead?
But that will take time, so I imagine a temporary workaround would be
useful to Ferenc.
> (the commit
> that broke things is part of 3.0 kernel so all current 3.x -stable
> kernels are affected)
[...]
The fix went into 3.3, so only 3.0 and 3.2 need it.
David, please can you include the above commit in your next batches for
these stable series?
Ben.
--
Ben Hutchings
For every complex problem
there is a solution that is simple, neat, and wrong.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply
* Re: [PATCH v2] iproute2: add support for tcp_metrics
From: Stephen Hemminger @ 2012-10-03 0:26 UTC (permalink / raw)
To: Julian Anastasov; +Cc: Eric Dumazet, netdev
In-Reply-To: <alpine.LFD.2.00.1210030302320.25856@ja.ssi.bg>
On Wed, 3 Oct 2012 03:21:08 +0300 (EEST)
Julian Anastasov <ja@ssi.bg> wrote:
> Something like this in process_msg?:
>
> case TCP_METRIC_RTT:
> fprintf(fp, "%lluus",
> ((__u64) val * 1000) >> 3);
> break;
> case TCP_METRIC_RTTVAR:
> fprintf(fp, "%lluus",
> ((__u64) val * 1000) >> 2);
> break;
>
> And may be variant without __u64 should be fine?
Remember u64 is not always same as unsigned long long.
One safe way of handling this is:
#include <inttypes.h>
...
case TCP_METRIC_RTT:
fprintf(fp, "%"PRIu64"us",
((__u64) val * 1000) >> 3);
break;
case TCP_METRIC_RTTVAR:
fprintf(fp, "%"PRIu64"us",
((__u64) val * 1000) >> 2);
break;
Also make sure metrics work independent of kernel HZ.
^ permalink raw reply
* Re: tg3 driver upgrade (Linux 2.6.32 -> 3.2) breaks IBM Bladecenter SoL
From: David Miller @ 2012-10-03 0:47 UTC (permalink / raw)
To: ben
Cc: mjt, wferi, mchan, netdev, mcarlson, grant.likely, rob.herring,
linux-kernel, stable
In-Reply-To: <1349223432.16173.10.camel@deadeye.wl.decadent.org.uk>
From: Ben Hutchings <ben@decadent.org.uk>
Date: Wed, 03 Oct 2012 01:17:12 +0100
> On Tue, 2012-10-02 at 23:06 +0400, Michael Tokarev wrote:
>> On 02.10.2012 22:49, Ferenc Wagner wrote:
>> > "Michael Chan" <mchan@broadcom.com> writes:
>> >> These are the likely fixes:
>> >>
>> >> commit cf9ecf4b631f649a964fa611f1a5e8874f2a76db
>> >> Author: Matt Carlson <mcarlson@broadcom.com>
>> >> Date: Mon Nov 28 09:41:03 2011 +0000
>> >>
>> >> tg3: Fix TSO CAP for 5704 devs w / ASF enabled
...
> The fix went into 3.3, so only 3.0 and 3.2 need it.
>
> David, please can you include the above commit in your next batches for
> these stable series?
Done.
^ permalink raw reply
* Re: [PATCH 1/2] Fix build error caused by broken PCH_PTP module dependency.
From: David Miller @ 2012-10-03 2:22 UTC (permalink / raw)
To: haicheng.li; +Cc: netdev, tshimizu818, linux-kernel, haicheng.lee
In-Reply-To: <50654A62.5090003@linux.intel.com>
From: Haicheng Li <haicheng.li@linux.intel.com>
Date: Fri, 28 Sep 2012 14:57:38 +0800
> On 09/28/2012 02:46 PM, David Miller wrote:
>> From: Haicheng Li<haicheng.li@linux.intel.com>
>> Date: Fri, 28 Sep 2012 14:41:43 +0800
>>
>>> On 09/28/2012 06:09 AM, David Miller wrote:
>>>> Look at how other people submit patches, do any other patch
>>>> submissions
>>>> look like your's having all of this metadata in the message body:
>>> I'm sorry for it.
>>>
>>>> As for this specific patch:
>>>>
>>>>> - depends on PTP_1588_CLOCK_PCH
>>>>> + depends on PTP_1588_CLOCK_PCH = PCH_GBE
>>>>
>>>> This is not the correct way to ensure that the module'ness of one
>>>> config option meets the module'ness requirements of another.
>>>> The correct way is to say something like "&& (PCH_GBE || PCH_GBE=n)"
>>>
>>> This case is a little bit tricky than usual, with PCH_PTP selected,
>>> the valid config would be either "PTP_1588_CLOCK_PCH=PCH_GBE=m" or
>>> "PTP_1588_CLOCK_PCH=PCH_GBE=y", and PTP_1588_CLOCK_PCH depends on
>>> PCH_GBE.
>>
>> And a simple "&& PCH_GBE" should accomplish this, no?
> No sir. it's actually same with the original Kconfig (by a if
> PCH_GBE"), it just failed with this config:
>
> CONFIG_PCH_GBE=y
> CONFIG_PCH_PTP=y
> CONFIG_PTP_1588_CLOCK=m
The correct fix is to make the Kconfig entry for PCH_PTP use
a "select PTP_1588_CLOCK" instead of "depends PTP_1588_CLOCK"
I'll apply this fix.
The is another, extremely convoluted, way to do this, which is
what the SFC driver does which is:
depends on SFC && PTP_1588_CLOCK && !(SFC=y && PTP_1588_CLOCK=m)
but that looks horrible to me.
^ permalink raw reply
* [PATCH] pch_gbe: Fix PTP dependencies.
From: David Miller @ 2012-10-03 2:35 UTC (permalink / raw)
To: netdev
The config combination:
CONFIG_PCH_GBE=y
CONFIG_PCH_PTP=y
CONFIG_PTP_1588_CLOCK=m
doesn't work, because then you have a built-in kernel
object (the PCH_PTP code) referring to symbols in a
module (PTP_1588_CLOCK).
Fix this like IXGBE, by using "select PTP_1588_CLOCK"
instead of a "depends on".
Reported-by: Haicheng Li <haicheng.li@linux.intel.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
drivers/net/ethernet/oki-semi/pch_gbe/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/Kconfig b/drivers/net/ethernet/oki-semi/pch_gbe/Kconfig
index bce0164..9730241 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/Kconfig
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/Kconfig
@@ -26,7 +26,7 @@ if PCH_GBE
config PCH_PTP
bool "PCH PTP clock support"
default n
- depends on PTP_1588_CLOCK_PCH
+ select PTP_1588_CLOCK_PCH
---help---
Say Y here if you want to use Precision Time Protocol (PTP) in the
driver. PTP is a method to precisely synchronize distributed clocks
--
1.7.11.4
^ permalink raw reply related
* Re: [PATCH] net: ethernet: davinci_cpdma: decrease the desc count when cleaning up the remaining packets
From: David Miller @ 2012-10-03 2:35 UTC (permalink / raw)
To: hotforest; +Cc: netdev, linux-kernel
In-Reply-To: <1349145763-6292-1-git-send-email-hotforest@gmail.com>
From: Tao Hou <hotforest@gmail.com>
Date: Tue, 2 Oct 2012 10:42:43 +0800
> chan->count is used by rx channel. If the desc count is not updated by
> the clean up loop in cpdma_chan_stop, the value written to the rxfree
> register in cpdma_chan_start will be incorrect.
>
> Signed-off-by: Tao Hou <hotforest@gmail.com>
Applied, thanks.
^ permalink raw reply
* 'net-net' is CLOSED
From: David Miller @ 2012-10-03 2:36 UTC (permalink / raw)
To: netdev
All changes are now going into the 'net' tree.
The 'net-next' tree will open up shortly after the
merge window close.
Do not submit net-next patches at this time, thanks.
^ permalink raw reply
* Re: [PATCH v2] ipv6: don't add link local route when there is no link local address
From: David Miller @ 2012-10-03 2:38 UTC (permalink / raw)
To: nicolas.dichtel; +Cc: netdev, yoshfuji
In-Reply-To: <1349169554-3774-1-git-send-email-nicolas.dichtel@6wind.com>
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Tue, 2 Oct 2012 11:19:14 +0200
> When an address is added on loopback (ip -6 a a 2002::1/128 dev lo), a route
> to fe80::/64 is added in the main table:
> unreachable fe80::/64 dev lo proto kernel metric 256 error -101
>
> This route does not match any prefix (no fe80:: address on lo). In fact,
> addrconf_dev_config() will not add link local address because this function
> filters interfaces by type. If the link local address is added manually, the
> route to the link local prefix will be automatically added by
> addrconf_add_linklocal().
> Note also, that this route is not deleted when the address is removed.
>
> After looking at the code, it seems that addrconf_add_lroute() is redundant with
> addrconf_add_linklocal(), because this function will add the link local route
> when the link local address is configured.
>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next v2] netxen: write IP address to firmware when using bonding
From: David Miller @ 2012-10-03 2:40 UTC (permalink / raw)
To: nikolay; +Cc: sony.chacko, agospoda, rajesh.borundia, netdev
In-Reply-To: <1349169386-18391-1-git-send-email-nikolay@redhat.com>
From: Nikolay Aleksandrov <nikolay@redhat.com>
Date: Tue, 2 Oct 2012 11:16:26 +0200
> This patch allows LRO aggregation on bonded devices that contain an NX3031
> device. It also adds a for_each_netdev_in_bond_rcu(bond, slave) macro
> which executes for each slave that has bond as master.
>
> V2: Remove local ip caching, retrieve addresses dynamically and
> restore them if necessary.
>
> Note: Tested with NX3031 adapter.
>
> Signed-off-by: Andy Gospodarek <agospoda@redhat.com>
> Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
This doesn't apply cleanly to the current tree.
It is also poorly styled, for example:
> + if (addr) {
> + netxen_config_ipaddr(adapter, addr,
> + ip_event);
> + }
No curley braces for a single statement.
There are also a lot of extransoue empty lines added to the
code which do not enhance readability in any way and just take
up valuable vertical screen realestate.
You've also missed the net-next merge window cutoff, so you'll
need to wait until after the merge window to send this in again.
Thanks.
^ permalink raw reply
* Re: [patch] bnx2x: use strlcpy() to copy a string
From: David Miller @ 2012-10-03 2:41 UTC (permalink / raw)
To: dan.carpenter; +Cc: eilong, netdev, kernel-janitors
In-Reply-To: <20121002114746.GB1413@elgon.mountain>
From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Tue, 2 Oct 2012 14:47:46 +0300
> DRV_MODULE_VERSION is smaller than the ->version buffer so the memcpy()
> copies 1 byte past the end of the string. It's not super harmful, but
> it makes the static checkers complain.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Applied, thanks Dan.
^ permalink raw reply
* Re: [PATCH net] 8021q: fix mac_len recomputation in vlan_untag()
From: David Miller @ 2012-10-03 2:46 UTC (permalink / raw)
To: ordex; +Cc: kaber, netdev
In-Reply-To: <1349194457-31623-1-git-send-email-ordex@autistici.org>
From: Antonio Quartulli <ordex@autistici.org>
Date: Tue, 2 Oct 2012 18:14:17 +0200
> skb_reset_mac_len() relies on the value of the skb->network_header pointer,
> therefore we must wait for such pointer to be recalculated before computing
> the new mac_len value.
>
> Signed-off-by: Antonio Quartulli <ordex@autistici.org>
Applied and queued up for -stable, thanks!
^ permalink raw reply
* Re: Possible networking regression in 3.6.0
From: David Miller @ 2012-10-03 2:55 UTC (permalink / raw)
To: eric.dumazet; +Cc: chris2553, netdev, gpiez, davej
In-Reply-To: <1349192919.12401.778.camel@edumazet-glaptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 02 Oct 2012 17:48:39 +0200
> [PATCH] ipv4: properly cache forward routes
>
> commit d2d68ba9fe8 (ipv4: Cache input routes in fib_info nexthops.)
> introduced a regression for forwarding.
>
> This was hard to reproduce but the symptom was that packets were
> delivered to local host instead of being forwarded.
>
> Add a separate cache (nh_rth_forward) to solve the problem.
>
> Many thanks to Chris Clayton for his patience and help.
>
> Reported-by: Chris Clayton <chris2553@googlemail.com>
> Bisected-by: Chris Clayton <chris2553@googlemail.com>
> Reported-by: Dave Jones <davej@redhat.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
I'm still having trouble understanding how this can happen,
which is probably why I introduced this bug in the first
place :-)
Only INPUT routes created by ip_route_input_slow() cache using
nh_rth_input.
Routes for locally destinations vs. forwarded destinations will
resolve to different fib_info objects.
If at some point a new route is added which turns a local destination
into one for which we forward, normal invalidation of cached routes
ought to fix it.
There's some sequence of events I don't understand that causes the
corrupt route cache, can you show it to me?
Thanks.
^ permalink raw reply
* Re: Possible networking regression in 3.6.0
From: David Miller @ 2012-10-03 3:10 UTC (permalink / raw)
To: ja; +Cc: eric.dumazet, chris2553, netdev, gpiez, davej
In-Reply-To: <alpine.LFD.2.00.1210030032430.25856@ja.ssi.bg>
From: Julian Anastasov <ja@ssi.bg>
Date: Wed, 3 Oct 2012 02:24:53 +0300 (EEST)
> Can it be a problem related to fib_info reuse
> from different routes. For example, when local IP address
> is created for subnet we have:
>
> broadcast 192.168.0.255 dev DEV proto kernel scope link src 192.168.0.1
> 192.168.0.0/24 dev DEV proto kernel scope link src 192.168.0.1
> local 192.168.0.1 dev DEV proto kernel scope host src 192.168.0.1
>
> The "dev DEV proto kernel scope link src 192.168.0.1" is
> a reused fib_info structure where we put cached routes.
> The result can be same fib_info for 192.168.0.255 and
> 192.168.0.0/24. RTN_BROADCAST is cached only for input
> routes. Incoming broadcast to 192.168.0.255 can be cached
> and can cause problems for traffic forwarded to 192.168.0.0/24.
> So, this patch should solve the problem because it
> separates the broadcast from unicast traffic.
Now I understand the problem.
I think the way to fix this is to add cfg->fc_type as another
thing that fib_info objects are key'd by.
I think it also would fix your obscure output multicast case too.
^ permalink raw reply
* [PATCH] tg3: Fix sparse warnings.
From: Michael Chan @ 2012-10-03 3:31 UTC (permalink / raw)
To: davem; +Cc: netdev, fengguang.wu
drivers/net/ethernet/broadcom/tg3.c:8121:8: warning: symbol 'i' shadows an earlier one
drivers/net/ethernet/broadcom/tg3.c:8003:6: originally declared here
drivers/net/ethernet/broadcom/tg3.c:785:5: warning: symbol 'tg3_ape_scratchpad_read' was not declared. Should it be static?
drivers/net/ethernet/broadcom/tg3.c:7781:19: warning: Using plain integer as NULL pointer
drivers/net/ethernet/broadcom/tg3.c:10231:31: error: bad constant expression
Reported-by: Fengguang Wu <fenguang.wu@intel.com>
Signed-off-by: Michael Chan <mchan@broadcom.com>
---
drivers/net/ethernet/broadcom/tg3.c | 11 ++++++-----
1 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 008ea14..b0624c2 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -782,7 +782,8 @@ static int tg3_ape_wait_for_event(struct tg3 *tp, u32 timeout_us)
return i == timeout_us / 10;
}
-int tg3_ape_scratchpad_read(struct tg3 *tp, u32 *data, u32 base_off, u32 len)
+static int tg3_ape_scratchpad_read(struct tg3 *tp, u32 *data, u32 base_off,
+ u32 len)
{
int err;
u32 i, bufoff, msgoff, maxlen, apedata;
@@ -7778,7 +7779,7 @@ static int tg3_alloc_consistent(struct tg3 *tp)
sblk = tnapi->hw_status;
if (tg3_flag(tp, ENABLE_RSS)) {
- u16 *prodptr = 0;
+ u16 *prodptr = NULL;
/*
* When RSS is enabled, the status block format changes
@@ -8118,11 +8119,11 @@ static int tg3_chip_reset(struct tg3 *tp)
u16 val16;
if (tp->pci_chip_rev_id == CHIPREV_ID_5750_A0) {
- int i;
+ int j;
u32 cfg_val;
/* Wait for link training to complete. */
- for (i = 0; i < 5000; i++)
+ for (j = 0; j < 5000; j++)
udelay(100);
pci_read_config_dword(tp->pdev, 0xc4, &cfg_val);
@@ -10228,7 +10229,7 @@ static u32 tg3_irq_count(struct tg3 *tp)
static bool tg3_enable_msix(struct tg3 *tp)
{
int i, rc;
- struct msix_entry msix_ent[tp->irq_max];
+ struct msix_entry msix_ent[TG3_IRQ_MAX_VECS];
tp->txq_cnt = tp->txq_req;
tp->rxq_cnt = tp->rxq_req;
--
1.6.4.GIT
^ permalink raw reply related
* Re: [PATCH] tg3: Fix sparse warnings.
From: David Miller @ 2012-10-03 3:22 UTC (permalink / raw)
To: mchan; +Cc: netdev, fengguang.wu
In-Reply-To: <1349235074-18927-1-git-send-email-mchan@broadcom.com>
From: "Michael Chan" <mchan@broadcom.com>
Date: Tue, 2 Oct 2012 20:31:14 -0700
> drivers/net/ethernet/broadcom/tg3.c:8121:8: warning: symbol 'i' shadows an earlier one
> drivers/net/ethernet/broadcom/tg3.c:8003:6: originally declared here
> drivers/net/ethernet/broadcom/tg3.c:785:5: warning: symbol 'tg3_ape_scratchpad_read' was not declared. Should it be static?
> drivers/net/ethernet/broadcom/tg3.c:7781:19: warning: Using plain integer as NULL pointer
> drivers/net/ethernet/broadcom/tg3.c:10231:31: error: bad constant expression
>
> Reported-by: Fengguang Wu <fenguang.wu@intel.com>
> Signed-off-by: Michael Chan <mchan@broadcom.com>
Applied, thanks.
^ permalink raw reply
* RE: [PATCHv4 1/4] modem_shm: Add Modem Access Framework
From: Arun MURTHY @ 2012-10-03 3:54 UTC (permalink / raw)
To: Greg KH
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-doc@vger.kernel.org, alan@lxorguk.ukuu.org.uk
In-Reply-To: <20121001155940.GA1957@kroah.com>
> On Mon, Oct 01, 2012 at 07:30:38AM +0200, Arun MURTHY wrote:
> > > On Fri, Sep 28, 2012 at 01:35:01PM +0530, Arun Murthy wrote:
> > > > +#include <linux/module.h>
> > > > +#include <linux/slab.h>
> > > > +#include <linux/err.h>
> > > > +#include <linux/printk.h>
> > > > +#include <linux/modem_shm/modem.h>
> > > > +
> > > > +static struct class *modem_class;
> > >
> > > What's wrong with a bus_type instead?
> >
> > Can I know the advantage of using bus_type over class?
>
> You have devices living on a bus, and it's much more descriptive than a class
> (which we are going to eventually get rid of one of these days...).
>
> Might I ask why you choose a class over a bus_type?
Basically my requirement is to create a central entity for accessing and releasing
modem from APE. Since this is done by different clients the central entity should
be able to handle the request and play safely, since this has more affect in
system suspend and deep sleep. Using class helps me in achieving this and
also create an entry to user space which can be used in the later parts. Moreover
this not something like a bus or so, so I didn't use bus instead went with a
simple class approach.
>
> > > > +int modem_release(struct modem_desc *mdesc) {
> > > > + if (!mdesc->release)
> > > > + return -EFAULT;
> > > > +
> > > > + if (modem_is_requested(mdesc)) {
> > > > + atomic_dec(&mdesc->mclients->cnt);
> > > > + if (atomic_read(&mdesc->use_cnt) == 1) {
> > > > + mdesc->release(mdesc);
> > > > + atomic_dec(&mdesc->use_cnt);
> > > > + }
> > >
> > > Eeek, why aren't you using the built-in reference counting that the
> > > struct device provided to you, and instead are rolling your own?
> > > This happens in many places, why?
> >
> > My usage of counters over here is for each modem there are many clients.
> > Each of the clients will have a ref to modem_desc. Each of them use
> > this for requesting and releasing the modem. One counter for tracking
> > the request and release for each client which is done by variable 'cnt' in
> struct clients.
> > The counter use_cnt is used for tracking the modem request/release
> > irrespective of the clients and counter cli_cnt is used for
> > restricting the modem_get to the no of clients defined in no_clients.
> >
> > So totally 3 counter one for restricting the usage of modem_get by
> > clients, second for restricting modem request/release at top level,
> > and 3rd for restricting modem release/request for per client per modem
> basis.
> >
> > Can you let me know if the same can be achieved by using built-in ref
> > counting?
>
> Yes, because you don't need all of those different levels, just stick with one
> and you should be fine. :)
>
No, checks at all these levels are required, I have briefed out the need also.
This will have effect on system power management, i.e suspend and deep
sleep.
We restrict that the drivers should request modem only once and release
only once, but we cannot rely on the clients hence a check for the same has
to be done in the MAF. Also the no of clients should be defined and hence a
check for the same is done in MAF. Apart from all these the requests coming
from all the clients is to be accumulated and based on that modem release
or access should be performed, hence so.
Thanks and Regards,
Arun R Murthy
-------------------
^ permalink raw reply
* Re: [PATCHv4 1/4] modem_shm: Add Modem Access Framework
From: anish singh @ 2012-10-03 4:17 UTC (permalink / raw)
To: Arun MURTHY
Cc: Greg KH, linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-doc@vger.kernel.org, alan@lxorguk.ukuu.org.uk
In-Reply-To: <F45880696056844FA6A73F415B568C695B6962CC8C@EXDCVYMBSTM006.EQ1STM.local>
On Wed, Oct 3, 2012 at 9:24 AM, Arun MURTHY <arun.murthy@stericsson.com> wrote:
>> On Mon, Oct 01, 2012 at 07:30:38AM +0200, Arun MURTHY wrote:
>> > > On Fri, Sep 28, 2012 at 01:35:01PM +0530, Arun Murthy wrote:
>> > > > +#include <linux/module.h>
>> > > > +#include <linux/slab.h>
>> > > > +#include <linux/err.h>
>> > > > +#include <linux/printk.h>
>> > > > +#include <linux/modem_shm/modem.h>
>> > > > +
>> > > > +static struct class *modem_class;
>> > >
>> > > What's wrong with a bus_type instead?
>> >
>> > Can I know the advantage of using bus_type over class?
>>
>> You have devices living on a bus, and it's much more descriptive than a class
>> (which we are going to eventually get rid of one of these days...).
>>
>> Might I ask why you choose a class over a bus_type?
>
> Basically my requirement is to create a central entity for accessing and releasing
> modem from APE. Since this is done by different clients the central entity should
> be able to handle the request and play safely, since this has more affect in
> system suspend and deep sleep. Using class helps me in achieving this and
> also create an entry to user space which can be used in the later parts. Moreover
You can have that same mechanism work for bus_type as well.
> this not something like a bus or so, so I didn't use bus instead went with a
> simple class approach.
>
>>
>> > > > +int modem_release(struct modem_desc *mdesc) {
>> > > > + if (!mdesc->release)
>> > > > + return -EFAULT;
>> > > > +
>> > > > + if (modem_is_requested(mdesc)) {
>> > > > + atomic_dec(&mdesc->mclients->cnt);
>> > > > + if (atomic_read(&mdesc->use_cnt) == 1) {
>> > > > + mdesc->release(mdesc);
>> > > > + atomic_dec(&mdesc->use_cnt);
>> > > > + }
>> > >
>> > > Eeek, why aren't you using the built-in reference counting that the
>> > > struct device provided to you, and instead are rolling your own?
>> > > This happens in many places, why?
>> >
>> > My usage of counters over here is for each modem there are many clients.
>> > Each of the clients will have a ref to modem_desc. Each of them use
>> > this for requesting and releasing the modem. One counter for tracking
>> > the request and release for each client which is done by variable 'cnt' in
>> struct clients.
>> > The counter use_cnt is used for tracking the modem request/release
>> > irrespective of the clients and counter cli_cnt is used for
>> > restricting the modem_get to the no of clients defined in no_clients.
>> >
>> > So totally 3 counter one for restricting the usage of modem_get by
>> > clients, second for restricting modem request/release at top level,
>> > and 3rd for restricting modem release/request for per client per modem
>> basis.
>> >
>> > Can you let me know if the same can be achieved by using built-in ref
>> > counting?
>>
>> Yes, because you don't need all of those different levels, just stick with one
>> and you should be fine. :)
>>
>
> No, checks at all these levels are required, I have briefed out the need also.
> This will have effect on system power management, i.e suspend and deep
> sleep.
> We restrict that the drivers should request modem only once and release
> only once, but we cannot rely on the clients hence a check for the same has
> to be done in the MAF. Also the no of clients should be defined and hence a
> check for the same is done in MAF. Apart from all these the requests coming
> from all the clients is to be accumulated and based on that modem release
> or access should be performed, hence so.
I think best way to deal with this is:
Define a new bus type and have your clients call the bus exposed functionality
when ever they need a service.So in your case it would be request and release
only AND when all of your clients have released the bus then you can do the
cleanup i.e. switch off the modem and on added advantage of making it a bus_type
would be that you can do the reference counting in your bus driver.
Designing is not my forte but I feel this way you can solve the problem at hand.
Please feel free to correct me.I would really appreciate it.
>
> Thanks and Regards,
> Arun R Murthy
> -------------------
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* RE: [PATCHv4 1/4] modem_shm: Add Modem Access Framework
From: Arun MURTHY @ 2012-10-03 4:31 UTC (permalink / raw)
To: anish singh
Cc: Greg KH, linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-doc@vger.kernel.org, alan@lxorguk.ukuu.org.uk
In-Reply-To: <CAK7N6vqpHvi0R4eY0NPJ90Yu1_sVXOri-qOU5FPhp_5XcY3z0w@mail.gmail.com>
> On Wed, Oct 3, 2012 at 9:24 AM, Arun MURTHY
> <arun.murthy@stericsson.com> wrote:
> >> On Mon, Oct 01, 2012 at 07:30:38AM +0200, Arun MURTHY wrote:
> >> > > On Fri, Sep 28, 2012 at 01:35:01PM +0530, Arun Murthy wrote:
> >> > > > +#include <linux/module.h>
> >> > > > +#include <linux/slab.h>
> >> > > > +#include <linux/err.h>
> >> > > > +#include <linux/printk.h>
> >> > > > +#include <linux/modem_shm/modem.h>
> >> > > > +
> >> > > > +static struct class *modem_class;
> >> > >
> >> > > What's wrong with a bus_type instead?
> >> >
> >> > Can I know the advantage of using bus_type over class?
> >>
> >> You have devices living on a bus, and it's much more descriptive than
> >> a class (which we are going to eventually get rid of one of these days...).
> >>
> >> Might I ask why you choose a class over a bus_type?
> >
> > Basically my requirement is to create a central entity for accessing
> > and releasing modem from APE. Since this is done by different clients
> > the central entity should be able to handle the request and play
> > safely, since this has more affect in system suspend and deep sleep.
> > Using class helps me in achieving this and also create an entry to
> > user space which can be used in the later parts. Moreover
> You can have that same mechanism work for bus_type as well.
> > this not something like a bus or so, so I didn't use bus instead went
> > with a simple class approach.
> >
> >>
> >> > > > +int modem_release(struct modem_desc *mdesc) {
> >> > > > + if (!mdesc->release)
> >> > > > + return -EFAULT;
> >> > > > +
> >> > > > + if (modem_is_requested(mdesc)) {
> >> > > > + atomic_dec(&mdesc->mclients->cnt);
> >> > > > + if (atomic_read(&mdesc->use_cnt) == 1) {
> >> > > > + mdesc->release(mdesc);
> >> > > > + atomic_dec(&mdesc->use_cnt);
> >> > > > + }
> >> > >
> >> > > Eeek, why aren't you using the built-in reference counting that
> >> > > the struct device provided to you, and instead are rolling your own?
> >> > > This happens in many places, why?
> >> >
> >> > My usage of counters over here is for each modem there are many
> clients.
> >> > Each of the clients will have a ref to modem_desc. Each of them use
> >> > this for requesting and releasing the modem. One counter for
> >> > tracking the request and release for each client which is done by
> >> > variable 'cnt' in
> >> struct clients.
> >> > The counter use_cnt is used for tracking the modem request/release
> >> > irrespective of the clients and counter cli_cnt is used for
> >> > restricting the modem_get to the no of clients defined in no_clients.
> >> >
> >> > So totally 3 counter one for restricting the usage of modem_get by
> >> > clients, second for restricting modem request/release at top level,
> >> > and 3rd for restricting modem release/request for per client per
> >> > modem
> >> basis.
> >> >
> >> > Can you let me know if the same can be achieved by using built-in
> >> > ref counting?
> >>
> >> Yes, because you don't need all of those different levels, just stick
> >> with one and you should be fine. :)
> >>
> >
> > No, checks at all these levels are required, I have briefed out the need also.
> > This will have effect on system power management, i.e suspend and deep
> > sleep.
> > We restrict that the drivers should request modem only once and
> > release only once, but we cannot rely on the clients hence a check for
> > the same has to be done in the MAF. Also the no of clients should be
> > defined and hence a check for the same is done in MAF. Apart from all
> > these the requests coming from all the clients is to be accumulated
> > and based on that modem release or access should be performed, hence
> so.
> I think best way to deal with this is:
> Define a new bus type and have your clients call the bus exposed
> functionality when ever they need a service.So in your case it would be
> request and release only AND when all of your clients have released the bus
> then you can do the cleanup i.e. switch off the modem and on added
> advantage of making it a bus_type would be that you can do the reference
> counting in your bus driver.
>
> Designing is not my forte but I feel this way you can solve the problem at
> hand.
> Please feel free to correct me.I would really appreciate it.
At the very first look itself this MAF is not a bus by its technical meaning, so
why to use bus_type is the point that I have.
Thanks and Regards,
Arun R Murthy
------------------
^ permalink raw reply
* Re: [PATCH 0/3] virtio-net: inline header support
From: Rusty Russell @ 2012-10-03 6:44 UTC (permalink / raw)
To: Michael S. Tsirkin, Thomas Lendacky
Cc: Sasha Levin, virtualization, linux-kernel, avi, kvm, netdev
In-Reply-To: <cover.1348824232.git.mst@redhat.com>
"Michael S. Tsirkin" <mst@redhat.com> writes:
> Thinking about Sasha's patches, we can reduce ring usage
> for virtio net small packets dramatically if we put
> virtio net header inline with the data.
> This can be done for free in case guest net stack allocated
> extra head room for the packet, and I don't see
> why would this have any downsides.
I've been wanting to do this for the longest time... but...
> Even though with my recent patches qemu
> no longer requires header to be the first s/g element,
> we need a new feature bit to detect this.
> A trivial qemu patch will be sent separately.
There's a reason I haven't done this. I really, really dislike "my
implemention isn't broken" feature bits. We could have an infinite
number of them, for each bug in each device.
So my plan was to tie this assumption to the new PCI layout. And have a
stress-testing patch like the one below in the kernel (see my virtio-wip
branch for stuff like this). Turn it on at boot with
"virtio_ring.torture" on the kernel commandline.
BTW, I've fixed lguest, but my kvm here (Ubuntu precise, kvm-qemu 1.0)
is too old. Building the latest git now...
Cheers,
Rusty.
Subject: virtio: CONFIG_VIRTIO_DEVICE_TORTURE
Virtio devices are not supposed to depend on the framing of the scatter-gather
lists, but various implementations did. Safeguard this in future by adding
an option to deliberately create perverse descriptors.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
index 8d5bddb..930a4ea 100644
--- a/drivers/virtio/Kconfig
+++ b/drivers/virtio/Kconfig
@@ -5,6 +5,15 @@ config VIRTIO
bus, such as CONFIG_VIRTIO_PCI, CONFIG_VIRTIO_MMIO, CONFIG_LGUEST,
CONFIG_RPMSG or CONFIG_S390_GUEST.
+config VIRTIO_DEVICE_TORTURE
+ bool "Virtio device torture tests"
+ depends on VIRTIO && DEBUG_KERNEL
+ help
+ This makes the virtio_ring implementation creatively change
+ the format of requests to make sure that devices are
+ properly implemented. This will make your virtual machine
+ slow *and* unreliable! Say N.
+
menu "Virtio drivers"
config VIRTIO_PCI
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index e639584..8893753 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -124,6 +124,149 @@ struct vring_virtqueue
#define to_vvq(_vq) container_of(_vq, struct vring_virtqueue, vq)
+#ifdef CONFIG_VIRTIO_DEVICE_TORTURE
+static bool torture;
+module_param(torture, bool, 0644);
+
+struct torture {
+ unsigned int orig_out, orig_in;
+ void *orig_data;
+ struct scatterlist sg[4];
+ struct scatterlist orig_sg[];
+};
+
+static size_t tot_len(struct scatterlist sg[], unsigned num)
+{
+ size_t len, i;
+
+ for (len = 0, i = 0; i < num; i++)
+ len += sg[i].length;
+
+ return len;
+}
+
+static void copy_sg_data(const struct scatterlist *dst, unsigned dnum,
+ const struct scatterlist *src, unsigned snum)
+{
+ unsigned len;
+ struct scatterlist s, d;
+
+ s = *src;
+ d = *dst;
+
+ while (snum && dnum) {
+ len = min(s.length, d.length);
+ memcpy(sg_virt(&d), sg_virt(&s), len);
+ d.offset += len;
+ d.length -= len;
+ s.offset += len;
+ s.length -= len;
+ if (!s.length) {
+ BUG_ON(snum == 0);
+ src++;
+ snum--;
+ s = *src;
+ }
+ if (!d.length) {
+ BUG_ON(dnum == 0);
+ dst++;
+ dnum--;
+ d = *dst;
+ }
+ }
+}
+
+static bool torture_replace(struct scatterlist **sg,
+ unsigned int *out,
+ unsigned int *in,
+ void **data,
+ gfp_t gfp)
+{
+ static size_t seed;
+ struct torture *t;
+ size_t outlen, inlen, ourseed, len1;
+ void *buf;
+
+ if (!torture)
+ return true;
+
+ outlen = tot_len(*sg, *out);
+ inlen = tot_len(*sg + *out, *in);
+
+ /* This will break horribly on large block requests. */
+ t = kmalloc(sizeof(*t) + (*out + *in) * sizeof(t->orig_sg[1])
+ + outlen + 1 + inlen + 1, gfp);
+ if (!t)
+ return false;
+
+ sg_init_table(t->sg, 4);
+ buf = &t->orig_sg[*out + *in];
+
+ memcpy(t->orig_sg, *sg, sizeof(**sg) * (*out + *in));
+ t->orig_out = *out;
+ t->orig_in = *in;
+ t->orig_data = *data;
+ *data = t;
+
+ ourseed = ACCESS_ONCE(seed);
+ seed++;
+
+ *sg = t->sg;
+ if (outlen) {
+ /* Split outbuf into two parts, one byte apart. */
+ *out = 2;
+ len1 = ourseed % (outlen + 1);
+ sg_set_buf(&t->sg[0], buf, len1);
+ buf += len1 + 1;
+ sg_set_buf(&t->sg[1], buf, outlen - len1);
+ buf += outlen - len1;
+ copy_sg_data(t->sg, *out, t->orig_sg, t->orig_out);
+ }
+
+ if (inlen) {
+ /* Split inbuf into two parts, one byte apart. */
+ *in = 2;
+ len1 = ourseed % (inlen + 1);
+ sg_set_buf(&t->sg[*out], buf, len1);
+ buf += len1 + 1;
+ sg_set_buf(&t->sg[*out + 1], buf, inlen - len1);
+ buf += inlen - len1;
+ }
+ return true;
+}
+
+static void *torture_done(struct torture *t)
+{
+ void *data;
+
+ if (!torture)
+ return t;
+
+ if (t->orig_in)
+ copy_sg_data(t->orig_sg + t->orig_out, t->orig_in,
+ t->sg + (t->orig_out ? 2 : 0), 2);
+
+ data = t->orig_data;
+ kfree(t);
+ return data;
+}
+
+#else
+static bool torture_replace(struct scatterlist **sg,
+ unsigned int *out,
+ unsigned int *in,
+ void **data,
+ gfp_t gfp)
+{
+ return true;
+}
+
+static void *torture_done(void *data)
+{
+ return data;
+}
+#endif /* CONFIG_VIRTIO_DEVICE_TORTURE */
+
/* Set up an indirect table of descriptors and add it to the queue. */
static int vring_add_indirect(struct vring_virtqueue *vq,
struct scatterlist sg[],
@@ -213,6 +356,9 @@ int virtqueue_add_buf(struct virtqueue *_vq,
BUG_ON(data == NULL);
+ if (!torture_replace(&sg, &out, &in, &data, gfp))
+ return -ENOMEM;
+
#ifdef DEBUG
{
ktime_t now = ktime_get();
@@ -246,6 +392,7 @@ int virtqueue_add_buf(struct virtqueue *_vq,
if (out)
vq->notify(&vq->vq);
END_USE(vq);
+ torture_done(data);
return -ENOSPC;
}
@@ -476,7 +623,7 @@ void *virtqueue_get_buf(struct virtqueue *_vq, unsigned int *len)
#endif
END_USE(vq);
- return ret;
+ return torture_done(ret);
}
EXPORT_SYMBOL_GPL(virtqueue_get_buf);
^ permalink raw reply related
* Re: [PATCH 0/3] virtio-net: inline header support
From: Rusty Russell @ 2012-10-03 7:10 UTC (permalink / raw)
To: Michael S. Tsirkin, Thomas Lendacky
Cc: Sasha Levin, virtualization, linux-kernel, avi, kvm, netdev
In-Reply-To: <87vces2gxq.fsf@rustcorp.com.au>
Rusty Russell <rusty@rustcorp.com.au> writes:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
>
>> Thinking about Sasha's patches, we can reduce ring usage
>> for virtio net small packets dramatically if we put
>> virtio net header inline with the data.
>> This can be done for free in case guest net stack allocated
>> extra head room for the packet, and I don't see
>> why would this have any downsides.
>
> I've been wanting to do this for the longest time... but...
>
>> Even though with my recent patches qemu
>> no longer requires header to be the first s/g element,
Breaks for me; see why I hate bug features? Now we'd need another
one...
qemu-system-i386: virtio: trying to map MMIO memory
Please try my patch.
Cheers,
Rusty.
^ permalink raw reply
* Re: [PATCH 0/2] PCI-Express Non-Transparent Bridge Support
From: Nicholas A. Bellinger @ 2012-10-03 7:11 UTC (permalink / raw)
To: Jon Mason; +Cc: linux-kernel, netdev, linux-pci, Dave Jiang, Christoph Hellwig
In-Reply-To: <1349213177-9985-1-git-send-email-jon.mason@intel.com>
On Tue, 2012-10-02 at 14:26 -0700, Jon Mason wrote:
> I am submitting version 4 of the PCI-Express Non-Transparent Bridge
> patches for inclusion in 3.7. All outstanding issues from the RFC
> process have been addressed.
>
> version 1
> http://thread.gmane.org/gmane.linux.kernel.pci/16443
>
> Version 2 incorporates numerous clean-ups
> http://thread.gmane.org/gmane.linux.kernel.pci/16696
>
> Version 3 incorporates changes to conform NTB and client devices to the
> Linux device model (per Greg KH's request).
> http://thread.gmane.org/gmane.linux.kernel.pci/17808
>
> Version 4 removes the transport transmit tasklet (per Dave Miller's
> request).
> http://thread.gmane.org/gmane.linux.network/244491
>
Hi Jon,
I'm really excited to see this series merged for v3.7-rc code.
Please feel free to add my:
Reviewed-by: Nicholas Bellinger <nab@linux-iscsi.org>
to the two individual patches for an initial merge. I'm not sure which
tree that your intending to take this code upstream via, but also feel
free to add my:
Acked-by: Nicholas Bellinger <nab@linux-iscsi.org>
of your maintainer-ship of /drivers/ntb/ subsystem code.
Very nice work on this series Jon !
--nab
^ permalink raw reply
* [PATCH] udp: increment UDP_MIB_NOPORTS in mcast receive
From: Eric Dumazet @ 2012-10-03 7:28 UTC (permalink / raw)
To: Julian Anastasov; +Cc: David Miller, chris2553, netdev, gpiez, Dave Jones
In-Reply-To: <alpine.LFD.2.00.1210030032430.25856@ja.ssi.bg>
On Wed, 2012-10-03 at 02:24 +0300, Julian Anastasov wrote:
> Hello,
>
> On Tue, 2 Oct 2012, Eric Dumazet wrote:
>
> > > David, shouldnt we use a nh_rth_forward instead of a nh_rth_input in
> > > __mkroute_input() ?
> > >
> > > (And change rt_cache_route() as well ?)
> > >
> > > I am testing a patch right now.
> >
> > Yeah, this patch seems to fix the bug for me.
> >
> > [PATCH] ipv4: properly cache forward routes
> >
> > commit d2d68ba9fe8 (ipv4: Cache input routes in fib_info nexthops.)
> > introduced a regression for forwarding.
> >
> > This was hard to reproduce but the symptom was that packets were
> > delivered to local host instead of being forwarded.
> >
> > Add a separate cache (nh_rth_forward) to solve the problem.
>
> Can it be a problem related to fib_info reuse
> from different routes. For example, when local IP address
> is created for subnet we have:
>
> broadcast 192.168.0.255 dev DEV proto kernel scope link src 192.168.0.1
> 192.168.0.0/24 dev DEV proto kernel scope link src 192.168.0.1
> local 192.168.0.1 dev DEV proto kernel scope host src 192.168.0.1
>
> The "dev DEV proto kernel scope link src 192.168.0.1" is
> a reused fib_info structure where we put cached routes.
> The result can be same fib_info for 192.168.0.255 and
> 192.168.0.0/24. RTN_BROADCAST is cached only for input
> routes. Incoming broadcast to 192.168.0.255 can be cached
> and can cause problems for traffic forwarded to 192.168.0.0/24.
> So, this patch should solve the problem because it
> separates the broadcast from unicast traffic.
>
> And the ip_route_input_slow caching will work for
> local and broadcast input routes (above routes 1 and 3) just
> because they differ in scope and use different fib_info.
>
> Another possible failure is for output routes:
>
> multicast 224.0.0.0/4 fib_info
> with unicast
> 192.168.0.0/24 fib_info
>
> The multicast sets RTCF_MULTICAST | RTCF_LOCAL
> and can cause problems for generated unicast traffic on
> fib_info reuse. Depends on the scope, for multicast it is
> usually scope global, so may be it is difficult to happen
> in practice.
>
> __mkroute_output works for local/unicast routes
> because they differ in scope.
Thanks Julian for these informations.
BTW, it seems we dont properly increase UDP MIB counters when a
multicast message is not delivered to at least one socket.
Lets fix this to ease future bug hunting.
I hate when "netstat -s" is useless and we have to use dropwatch to
figure out where we drop a frame.
[PATCH] udp: increment UDP_MIB_NOPORTS in multicast receive
We should increment UDP_MIB_NOPORTS in the case we found
no socket to deliver a copy of one incoming UDP message.
(RFC 4113 udpNoPorts)
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/udp.c | 1 +
net/ipv6/udp.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 79c8dbe..dfa73c5 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1591,6 +1591,7 @@ static int __udp4_lib_mcast_deliver(struct net *net, struct sk_buff *skb,
sock_put(stack[i]);
} else {
kfree_skb(skb);
+ UDP_INC_STATS_BH(net, UDP_MIB_NOPORTS, udptable != &udp_table);
}
return 0;
}
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index fc99972..0be9ac2 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -748,6 +748,7 @@ static int __udp6_lib_mcast_deliver(struct net *net, struct sk_buff *skb,
sock_put(stack[i]);
} else {
kfree_skb(skb);
+ UDP6_INC_STATS_BH(net, UDP_MIB_NOPORTS, udptable != &udp_table);
}
return 0;
}
^ permalink raw reply related
* [PATCH] iputils: ping: Fix typo in echo reply
From: Jan Synacek @ 2012-10-03 8:26 UTC (permalink / raw)
To: yoshfuji; +Cc: netdev
[-- Attachment #1: Type: text/plain, Size: 136 bytes --]
Hello,
here is a fix for a typo that's currently present in ping.
Cheers,
--
Jan Synacek
Software Engineer, BaseOS team Brno, Red Hat
[-- Attachment #2: 0001-ping-Fix-typo-in-echo-reply.patch --]
[-- Type: text/x-patch, Size: 662 bytes --]
>From 3b5623a03279538ca9a6f3796626c1aad9ef5477 Mon Sep 17 00:00:00 2001
From: Jan Synacek <jsynacek@redhat.com>
Date: Wed, 3 Oct 2012 10:14:45 +0200
Subject: [PATCH] ping: Fix typo in echo reply
Signed-off-by: Jan Synacek <jsynacek@redhat.com>
---
ping.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ping.c b/ping.c
index 1425d1d..3ab2ae0 100644
--- a/ping.c
+++ b/ping.c
@@ -705,7 +705,7 @@ int send_probe()
void pr_echo_reply(__u8 *_icp, int len)
{
struct icmphdr *icp = (struct icmphdr *)_icp;
- printf(" icmp_req=%u", ntohs(icp->un.echo.sequence));
+ printf(" icmp_seq=%u", ntohs(icp->un.echo.sequence));
}
int
--
1.7.11.4
^ permalink raw reply related
* RE: [PATCH v2] iproute2: add support for tcp_metrics
From: David Laight @ 2012-10-03 8:20 UTC (permalink / raw)
To: Stephen Hemminger, Julian Anastasov; +Cc: Eric Dumazet, netdev
In-Reply-To: <20121002172633.2be06886@nehalam.linuxnetplumber.net>
> > fprintf(fp, "%lluus",
> > ((__u64) val * 1000) >> 2);
...
> Remember u64 is not always same as unsigned long long.
> One safe way of handling this is:
...
> fprintf(fp, "%"PRIu64"us",
> ((__u64) val * 1000) >> 3);
Or convert the constant.
fprintf(fp, "%lluus", (val * 1000ull) >> 2);
which may well generate better code on 32bit systems.
David
^ permalink raw reply
* Re: [PATCH net-next v2] netxen: write IP address to firmware when using bonding
From: Nikolay Aleksandrov @ 2012-10-03 9:20 UTC (permalink / raw)
To: sony.chacko; +Cc: agospoda, rajesh.borundia, davem, netdev
In-Reply-To: <1349169386-18391-1-git-send-email-nikolay@redhat.com>
On 10/02/2012 11:16 AM, Nikolay Aleksandrov wrote:
> This patch allows LRO aggregation on bonded devices that contain an NX3031
> device. It also adds a for_each_netdev_in_bond_rcu(bond, slave) macro
> which executes for each slave that has bond as master.
>
> V2: Remove local ip caching, retrieve addresses dynamically and
> restore them if necessary.
>
> Note: Tested with NX3031 adapter.
>
> Signed-off-by: Andy Gospodarek <agospoda@redhat.com>
> Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
> ---
> drivers/net/ethernet/qlogic/netxen/netxen_nic.h | 6 -
> .../net/ethernet/qlogic/netxen/netxen_nic_main.c | 192 +++++++++++----------
> include/linux/netdevice.h | 3 +
> 3 files changed, 100 insertions(+), 101 deletions(-)
>
> diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic.h b/drivers/net/ethernet/qlogic/netxen/netxen_nic.h
> index eb3dfdb..cb4f2ce 100644
> --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic.h
> +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic.h
> @@ -955,11 +955,6 @@ typedef struct nx_mac_list_s {
> uint8_t mac_addr[ETH_ALEN+2];
> } nx_mac_list_t;
>
> -struct nx_vlan_ip_list {
> - struct list_head list;
> - __be32 ip_addr;
> -};
> -
> /*
> * Interrupt coalescing defaults. The defaults are for 1500 MTU. It is
> * adjusted based on configured MTU.
> @@ -1605,7 +1600,6 @@ struct netxen_adapter {
> struct net_device *netdev;
> struct pci_dev *pdev;
> struct list_head mac_list;
> - struct list_head vlan_ip_list;
>
> spinlock_t tx_clean_lock;
>
> diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
> index e2a4858..8e3eb61 100644
> --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
> +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
> @@ -90,7 +90,6 @@ static irqreturn_t netxen_intr(int irq, void *data);
> static irqreturn_t netxen_msi_intr(int irq, void *data);
> static irqreturn_t netxen_msix_intr(int irq, void *data);
>
> -static void netxen_free_vlan_ip_list(struct netxen_adapter *);
> static void netxen_restore_indev_addr(struct net_device *dev, unsigned long);
> static struct rtnl_link_stats64 *netxen_nic_get_stats(struct net_device *dev,
> struct rtnl_link_stats64 *stats);
> @@ -1451,7 +1450,6 @@ netxen_nic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>
> spin_lock_init(&adapter->tx_clean_lock);
> INIT_LIST_HEAD(&adapter->mac_list);
> - INIT_LIST_HEAD(&adapter->vlan_ip_list);
>
> err = netxen_setup_pci_map(adapter);
> if (err)
> @@ -1586,7 +1584,7 @@ static void __devexit netxen_nic_remove(struct pci_dev *pdev)
>
> cancel_work_sync(&adapter->tx_timeout_task);
>
> - netxen_free_vlan_ip_list(adapter);
> + netxen_restore_indev_addr(netdev, NETDEV_DOWN);
> netxen_nic_detach(adapter);
>
> nx_decr_dev_ref_cnt(adapter);
> @@ -3135,66 +3133,22 @@ netxen_destip_supported(struct netxen_adapter *adapter)
> return 1;
> }
>
> -static void
> -netxen_free_vlan_ip_list(struct netxen_adapter *adapter)
> +static inline __be32
> +netxen_get_addr(struct net_device *dev)
> {
> - struct nx_vlan_ip_list *cur;
> - struct list_head *head = &adapter->vlan_ip_list;
> -
> - while (!list_empty(head)) {
> - cur = list_entry(head->next, struct nx_vlan_ip_list, list);
> - netxen_config_ipaddr(adapter, cur->ip_addr, NX_IP_DOWN);
> - list_del(&cur->list);
> - kfree(cur);
> - }
> -
> -}
> -static void
> -netxen_list_config_vlan_ip(struct netxen_adapter *adapter,
> - struct in_ifaddr *ifa, unsigned long event)
> -{
> - struct net_device *dev;
> - struct nx_vlan_ip_list *cur, *tmp_cur;
> - struct list_head *head;
> -
> - dev = ifa->ifa_dev ? ifa->ifa_dev->dev : NULL;
> -
> - if (dev == NULL)
> - return;
> -
> - if (!is_vlan_dev(dev))
> - return;
> + struct in_device *in_dev;
> + __be32 addr = 0;
>
> - switch (event) {
> - case NX_IP_UP:
> - list_for_each(head, &adapter->vlan_ip_list) {
> - cur = list_entry(head, struct nx_vlan_ip_list, list);
> + rcu_read_lock();
> + in_dev = __in_dev_get_rcu(dev);
>
> - if (cur->ip_addr == ifa->ifa_address)
> - return;
> - }
> + if (in_dev)
> + addr = inet_confirm_addr(in_dev, 0, 0, RT_SCOPE_HOST);
>
> - cur = kzalloc(sizeof(struct nx_vlan_ip_list), GFP_ATOMIC);
> - if (cur == NULL) {
> - printk(KERN_ERR "%s: failed to add vlan ip to list\n",
> - adapter->netdev->name);
> - return;
> - }
> -
> - cur->ip_addr = ifa->ifa_address;
> - list_add_tail(&cur->list, &adapter->vlan_ip_list);
> - break;
> - case NX_IP_DOWN:
> - list_for_each_entry_safe(cur, tmp_cur,
> - &adapter->vlan_ip_list, list) {
> - if (cur->ip_addr == ifa->ifa_address) {
> - list_del(&cur->list);
> - kfree(cur);
> - break;
> - }
> - }
> - }
> + rcu_read_unlock();
> + return addr;
> }
> +
> static void
> netxen_config_indev_addr(struct netxen_adapter *adapter,
> struct net_device *dev, unsigned long event)
> @@ -3213,12 +3167,10 @@ netxen_config_indev_addr(struct netxen_adapter *adapter,
> case NETDEV_UP:
> netxen_config_ipaddr(adapter,
> ifa->ifa_address, NX_IP_UP);
> - netxen_list_config_vlan_ip(adapter, ifa, NX_IP_UP);
> break;
> case NETDEV_DOWN:
> netxen_config_ipaddr(adapter,
> ifa->ifa_address, NX_IP_DOWN);
> - netxen_list_config_vlan_ip(adapter, ifa, NX_IP_DOWN);
> break;
> default:
> break;
> @@ -3233,15 +3185,54 @@ netxen_restore_indev_addr(struct net_device *netdev, unsigned long event)
>
> {
> struct netxen_adapter *adapter = netdev_priv(netdev);
> - struct nx_vlan_ip_list *pos, *tmp_pos;
> + struct net_device *vlan_dev, *real_dev;
> unsigned long ip_event;
> + __be32 addr = 0;
>
> ip_event = (event == NETDEV_UP) ? NX_IP_UP : NX_IP_DOWN;
> netxen_config_indev_addr(adapter, netdev, event);
>
> - list_for_each_entry_safe(pos, tmp_pos, &adapter->vlan_ip_list, list) {
> - netxen_config_ipaddr(adapter, pos->ip_addr, ip_event);
> + rcu_read_lock();
> + for_each_netdev_rcu(&init_net, vlan_dev) {
> + if (vlan_dev->priv_flags & IFF_802_1Q_VLAN) {
> + real_dev = vlan_dev_real_dev(vlan_dev);
> +
> + if (real_dev == netdev) {
> + addr = netxen_get_addr(vlan_dev);
> +
> + if (addr) {
> + netxen_config_ipaddr(adapter, addr,
> + ip_event);
> + }
> + }
> + }
> }
> +
> + if (netdev->master) {
> + addr = netxen_get_addr(netdev->master);
> + if (addr)
> + netxen_config_ipaddr(adapter, addr, ip_event);
> + }
> + rcu_read_unlock();
> +}
> +
> +static inline int
> +netxen_config_checkdev(struct net_device *dev)
> +{
> + struct netxen_adapter *adapter;
> +
> + if (!is_netxen_netdev(dev))
> + return -ENODEV;
> +
> + adapter = netdev_priv(dev);
> +
> + if (!adapter)
> + return -ENODEV;
> +
> + if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
> + return -ENODEV;
> +
> + return 0;
> }
>
> static int netxen_netdev_event(struct notifier_block *this,
> @@ -3260,18 +3251,26 @@ recheck:
> goto recheck;
> }
>
> - if (!is_netxen_netdev(dev))
> - goto done;
> + /* If this is a bonding device, look for netxen-based slaves*/
> + if (dev->priv_flags & IFF_BONDING) {
> + struct net_device *slave;
>
> - adapter = netdev_priv(dev);
> + rcu_read_lock();
> + for_each_netdev_in_bond_rcu(dev, slave) {
> + if (netxen_config_checkdev(slave) < 0)
> + continue;
>
> - if (!adapter)
> - goto done;
> -
> - if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
> - goto done;
> + adapter = netdev_priv(slave);
> + netxen_config_indev_addr(adapter, orig_dev, event);
> + }
> + rcu_read_unlock();
> + } else {
> + if (netxen_config_checkdev(dev) < 0)
> + goto done;
>
> - netxen_config_indev_addr(adapter, orig_dev, event);
> + adapter = netdev_priv(dev);
> + netxen_config_indev_addr(adapter, orig_dev, event);
> + }
> done:
> return NOTIFY_DONE;
> }
> @@ -3282,10 +3281,11 @@ netxen_inetaddr_event(struct notifier_block *this,
> {
> struct netxen_adapter *adapter;
> struct net_device *dev;
> -
> + unsigned long ip_event;
> struct in_ifaddr *ifa = (struct in_ifaddr *)ptr;
>
> dev = ifa->ifa_dev ? ifa->ifa_dev->dev : NULL;
> + ip_event = (event == NETDEV_UP) ? NX_IP_UP : NX_IP_DOWN;
>
> recheck:
> if (dev == NULL)
> @@ -3296,30 +3296,35 @@ recheck:
> goto recheck;
> }
>
> - if (!is_netxen_netdev(dev))
> - goto done;
> + /* If this is a bonding device, look for netxen-based slaves*/
> + if (dev->priv_flags & IFF_BONDING) {
> + struct net_device *slave;
>
> - adapter = netdev_priv(dev);
> + rcu_read_lock();
> + for_each_netdev_in_bond_rcu(dev, slave) {
> + if (netxen_config_checkdev(slave) < 0)
> + continue;
>
> - if (!adapter || !netxen_destip_supported(adapter))
> - goto done;
> + adapter = netdev_priv(slave);
>
> - if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
> - goto done;
> + if (!netxen_destip_supported(adapter))
> + continue;
>
> - switch (event) {
> - case NETDEV_UP:
> - netxen_config_ipaddr(adapter, ifa->ifa_address, NX_IP_UP);
> - netxen_list_config_vlan_ip(adapter, ifa, NX_IP_UP);
> - break;
> - case NETDEV_DOWN:
> - netxen_config_ipaddr(adapter, ifa->ifa_address, NX_IP_DOWN);
> - netxen_list_config_vlan_ip(adapter, ifa, NX_IP_DOWN);
> - break;
> - default:
> - break;
> - }
> + netxen_config_ipaddr(adapter, ifa->ifa_address,
> + ip_event);
> + }
> + rcu_read_unlock();
> + } else {
> + if (netxen_config_checkdev(dev) < 0)
> + goto done;
> +
> + adapter = netdev_priv(dev);
>
> + if (!netxen_destip_supported(adapter))
> + goto done;
> +
> + netxen_config_ipaddr(adapter, ifa->ifa_address, ip_event);
> + }
> done:
> return NOTIFY_DONE;
> }
> @@ -3335,9 +3340,6 @@ static struct notifier_block netxen_inetaddr_cb = {
> static void
> netxen_restore_indev_addr(struct net_device *dev, unsigned long event)
> { }
> -static void
> -netxen_free_vlan_ip_list(struct netxen_adapter *adapter)
> -{ }
> #endif
>
> static struct pci_error_handlers netxen_err_handler = {
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 59dc05f3..463bb40 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -1578,6 +1578,9 @@ extern rwlock_t dev_base_lock; /* Device list lock */
> list_for_each_entry_continue(d, &(net)->dev_base_head, dev_list)
> #define for_each_netdev_continue_rcu(net, d) \
> list_for_each_entry_continue_rcu(d, &(net)->dev_base_head, dev_list)
> +#define for_each_netdev_in_bond_rcu(bond, slave) \
> + for_each_netdev_rcu(&init_net, slave) \
> + if (slave->master == bond)
> #define net_device_entry(lh) list_entry(lh, struct net_device, dev_list)
>
> static inline struct net_device *next_net_device(struct net_device *dev)
Hello Dave,
I just synced with upstream, I've missed a few patches and it seems
that it doesn't apply cleanly because my previous patch was
changed before it was applied. There is one character missing from
a comment - "/* root bus? */", in upstream it was changed to
/* root bus */.
("netxen: check for root bus in netxen_mask_aer_correctable")
About the rest, after QLogic test the functionality I'll clean-up the
empty lines and re-send it.
Thank you,
Nik
^ 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