* [RFT] sky2: turn on pci power
From: Stephen Hemminger @ 2007-08-07 19:12 UTC (permalink / raw)
To: Florian Lohoff; +Cc: Michal Piotrowski, netdev
In-Reply-To: <20070725072202.GA4905@paradigm.rfc822.org>
This setup step got dropped in 2.6.23, Yukon-EX configuration, maybe
this fixes your problem?
--- a/drivers/net/sky2.c 2007-08-06 04:39:36.000000000 -0400
+++ b/drivers/net/sky2.c 2007-08-07 14:50:25.000000000 -0400
@@ -222,6 +222,8 @@ static void sky2_power_on(struct sky2_hw
if (hw->chip_id == CHIP_ID_YUKON_EC_U || hw->chip_id == CHIP_ID_YUKON_EX) {
u32 reg;
+ sky2_pci_write32(hw, PCI_DEV_REG3, 0);
+
reg = sky2_pci_read32(hw, PCI_DEV_REG4);
/* set all bits to 0 except bits 15..12 and 8 */
reg &= P_ASPM_CONTROL_MSK;
^ permalink raw reply
* Re: sky2: workaround for lost IRQ and 2.6.22-stable
From: Stephen Hemminger @ 2007-08-07 19:14 UTC (permalink / raw)
To: Krzysztof Oledzki; +Cc: stable, netdev
In-Reply-To: <Pine.LNX.4.64.0708072043340.16599@bizon.gios.gov.pl>
On Tue, 7 Aug 2007 20:46:31 +0200 (CEST)
Krzysztof Oledzki <olel@ans.pl> wrote:
> Hello,
>
> http://git.kernel.org/?p=linux/kernel/git/stable/linux-2.6.21.y.git;a=commitdiff;h=fe1fe7c982f86624c692644e8ed05e132f4753cc
>
> Is this fix going to be included in the next 2.6.22-stable release or is
> it not needed any more?
>
> Best regards,
>
> Krzysztof Olędzki
It stops the major hang from IRQ loss. 2.6.23 has more minor stuff that
probably aren't needed for stablilty
^ permalink raw reply
* sky2: workaround for lost IRQ and 2.6.22-stable
From: Krzysztof Oledzki @ 2007-08-07 18:46 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: stable, netdev
[-- Attachment #1: Type: TEXT/PLAIN, Size: 278 bytes --]
Hello,
http://git.kernel.org/?p=linux/kernel/git/stable/linux-2.6.21.y.git;a=commitdiff;h=fe1fe7c982f86624c692644e8ed05e132f4753cc
Is this fix going to be included in the next 2.6.22-stable release or is
it not needed any more?
Best regards,
Krzysztof Olędzki
^ permalink raw reply
* Linx
From: Stephen Hemminger @ 2007-08-07 19:39 UTC (permalink / raw)
To: netdev
After seeing this article on Linx
http://www.linuxdevices.com/news/NS8613439087.html
I decided to give it a quick code review (long airline flight).
Overall, it isn't awful, it just looks like every other piece of code
that hasn't been managed for mainline kernel inclusion.
Nice way of saying, this turd needs a man year or more of polishing.
Gratiutious Code Review of Linx
0. Bugs.
A. Device names can change in kernel at anytime, use pointers
or ifindex. In fact any name change will crash kernel in
BUG_ON in notifier
B. Device's changing MTU will crash kernel in BUG_ON
C. Calling del_timer_sync under RTNL
1. Coding Style
A. Typedef's
Don't use typedef's like LINX_SPID, ...
B. Non-standard naming conventions
I. Don't use uint32_t for kernel use u32 or __u32
II. No MixedCaseNames
C. Use std. macros
I. BUG_ON vs. LINX_ASSERT, etc
D. Code in macro's that should really be inline's
(e.g. linx_check_linx_huntname)
E. Indentation
F. Excessive scope, much of the code could be local to one file
G. Too many spelling errors
H. OS Abstraction layer is unacceptable
I. Use initializers when possible (e.g device_notifier)
J. Quit with all the assert's for in_irq() in timer's etc...
2. Bogus wrappers
A. Kmalloc
B. Spinlocks
3. Unacceptable ABI
A. ioctl's for special functions
B. Heavy reliance on config parameters in /proc
C. Looks dependent on Ethernet address format
D. Code for non-standard adaptive coalesce
and his code has protocol playing with drivers timers directly.
E. Non-assigned number for Ethernet protocol
4. FYI
A. No __init or __exit
B. Kernel API documentation
Only document API calls that matter not every pissant little function.
Avoid stating the obvious.
Why not use docbook format?
C. Locking way to fine grained (lots of small locks)
Should use RCU and avoid rwlocks
Use existing linux network device API locks (ie dev_base_lock, RTNL)
if possible.
Those who don't understand TCP/IP are doomed to reimplement it, badly.
^ permalink raw reply
* Re: [stable] sky2: workaround for lost IRQ and 2.6.22-stable
From: Greg KH @ 2007-08-07 18:56 UTC (permalink / raw)
To: Krzysztof Oledzki; +Cc: Stephen Hemminger, netdev, stable
In-Reply-To: <Pine.LNX.4.64.0708072043340.16599@bizon.gios.gov.pl>
On Tue, Aug 07, 2007 at 08:46:31PM +0200, Krzysztof Oledzki wrote:
> Hello,
>
> http://git.kernel.org/?p=linux/kernel/git/stable/linux-2.6.21.y.git;a=commitdiff;h=fe1fe7c982f86624c692644e8ed05e132f4753cc
>
> Is this fix going to be included in the next 2.6.22-stable release or is it
> not needed any more?
It's not queued up for the next 2.6.22-stable release as no one has sent
it to the stable maintainers :)
thanks,
greg k-h
^ permalink raw reply
* Re: Distributed storage.
From: Jens Axboe @ 2007-08-07 20:55 UTC (permalink / raw)
To: Daniel Phillips
Cc: Evgeniy Polyakov, netdev, linux-kernel, linux-fsdevel,
Peter Zijlstra
In-Reply-To: <200708071124.56859.phillips@phunq.net>
On Tue, Aug 07 2007, Daniel Phillips wrote:
> On Tuesday 07 August 2007 05:05, Jens Axboe wrote:
> > On Sun, Aug 05 2007, Daniel Phillips wrote:
> > > A simple way to solve the stable accounting field issue is to add a
> > > new pointer to struct bio that is owned by the top level submitter
> > > (normally generic_make_request but not always) and is not affected
> > > by any recursive resubmission. Then getting rid of that field
> > > later becomes somebody's summer project, which is not all that
> > > urgent because struct bio is already bloated up with a bunch of
> > > dubious fields and is a transient structure anyway.
> >
> > Thanks for your insights. Care to detail what bloat and dubious
> > fields struct bio has?
>
> First obvious one I see is bi_rw separate from bi_flags. Front_size and
> back_size smell dubious. Is max_vecs really necessary? You could
I don't like structure bloat, but I do like nice design. Overloading is
a necessary evil sometimes, though. Even today, there isn't enough room
to hold bi_rw and bi_flags in the same variable on 32-bit archs, so that
concern can be scratched. If you read bio.h, that much is obvious.
If you check up on the iommu virtual merging, you'll understand the
front and back size members. They may smell dubious to you, but please
take the time to understand why it looks the way it does.
> reasonably assume bi_vcnt rounded up to a power of two and bury the
> details of making that work behind wrapper functions to change the
> number of bvecs, if anybody actually needs that. Bi_endio and
Changing the number of bvecs is integral to how bio buildup current
works.
> bi_destructor could be combined. I don't see a lot of users of bi_idx,
bi_idx is integral to partial io completions.
> that looks like a soft target. See what happened to struct page when a
> couple of folks got serious about attacking it, some really deep hacks
> were done to pare off a few bytes here and there. But struct bio as a
> space waster is not nearly in the same ballpark.
So show some concrete patches and examples, hand waving and assumptions
is just a waste of everyones time.
> It would be interesting to see if bi_bdev could be made read only.
> Generally, each stage in the block device stack knows what the next
> stage is going to be, so why do we have to write that in the bio? For
> error reporting from interrupt context? Anyway, if Evgeniy wants to do
> the patch, I will happily unload the task of convincing you that random
> fields are/are not needed in struct bio :-)
It's a trade off, otherwise you'd have to pass the block device around a
lot. And it's, again, a design issue. A bio contains destination
information, that means device/offset/size information. I'm all for
shaving structure bytes where it matters, but not for the sake of
sacrificing code stability or design. I consider struct bio quite lean
and have worked hard to keep it that way. In fact, iirc, the only
addition to struct bio since 2001 is the iommu front/back size members.
And I resisted those for quite a while.
--
Jens Axboe
^ permalink raw reply
* [patch 1/1] NetLabel: add missing rcu_dereference() calls in the LSM domain mapping hash table
From: Paul Moore @ 2007-08-07 20:54 UTC (permalink / raw)
To: netdev; +Cc: Paul Moore
h
Content-Disposition: inline; filename=netlabel-rcu_deref_fix
The LSM domain mapping head table pointer was not being referenced via the RCU
safe dereferencing function, rcu_dereference(). This patch adds those missing
calls to the NetLabel code.
This has been tested using recent linux-2.6 git kernels with no visible
regressions.
Signed-off-by: Paul Moore <paul.moore@hp.com>
---
net/netlabel/netlabel_domainhash.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
Index: linux-2.6_nlbl-rcu-fixup/net/netlabel/netlabel_domainhash.c
===================================================================
--- linux-2.6_nlbl-rcu-fixup.orig/net/netlabel/netlabel_domainhash.c
+++ linux-2.6_nlbl-rcu-fixup/net/netlabel/netlabel_domainhash.c
@@ -126,7 +126,9 @@ static struct netlbl_dom_map *netlbl_dom
if (domain != NULL) {
bkt = netlbl_domhsh_hash(domain);
- list_for_each_entry_rcu(iter, &netlbl_domhsh->tbl[bkt], list)
+ list_for_each_entry_rcu(iter,
+ &rcu_dereference(netlbl_domhsh)->tbl[bkt],
+ list)
if (iter->valid && strcmp(iter->domain, domain) == 0)
return iter;
}
@@ -227,7 +229,7 @@ int netlbl_domhsh_add(struct netlbl_dom_
spin_lock(&netlbl_domhsh_lock);
if (netlbl_domhsh_search(entry->domain, 0) == NULL)
list_add_tail_rcu(&entry->list,
- &netlbl_domhsh->tbl[bkt]);
+ &rcu_dereference(netlbl_domhsh)->tbl[bkt]);
else
ret_val = -EEXIST;
spin_unlock(&netlbl_domhsh_lock);
@@ -423,8 +425,8 @@ int netlbl_domhsh_walk(u32 *skip_bkt,
iter_bkt < rcu_dereference(netlbl_domhsh)->size;
iter_bkt++, chain_cnt = 0) {
list_for_each_entry_rcu(iter_entry,
- &netlbl_domhsh->tbl[iter_bkt],
- list)
+ &rcu_dereference(netlbl_domhsh)->tbl[iter_bkt],
+ list)
if (iter_entry->valid) {
if (chain_cnt++ < *skip_chain)
continue;
--
paul moore
linux security @ hp
^ permalink raw reply
* [RFT] sky2: backport patch
From: Stephen Hemminger @ 2007-08-07 21:14 UTC (permalink / raw)
To: Greg KH; +Cc: Krzysztof Oledzki, netdev, stable
In-Reply-To: <20070807185631.GA19140@kroah.com>
Any volunteers to test this, it has a backport for the three main stability patches:
1. carrier management
2. lost irq timer
3. rechecking for packets in poll
4. overlength packet hang.
I am away from any sky2 hardware for another week, but others maybe
able to validate this.
--- a/drivers/net/sky2.c 2007-07-20 04:30:14.000000000 -0400
+++ b/drivers/net/sky2.c 2007-08-07 17:08:21.000000000 -0400
@@ -96,7 +96,7 @@ static int disable_msi = 0;
module_param(disable_msi, int, 0);
MODULE_PARM_DESC(disable_msi, "Disable Message Signaled Interrupt (MSI)");
-static int idle_timeout = 0;
+static int idle_timeout = 100;
module_param(idle_timeout, int, 0);
MODULE_PARM_DESC(idle_timeout, "Watchdog timer for lost interrupts (ms)");
@@ -1234,6 +1234,8 @@ static int sky2_up(struct net_device *de
if (netif_msg_ifup(sky2))
printk(KERN_INFO PFX "%s: enabling interface\n", dev->name);
+ netif_carrier_off(dev);
+
/* must be power of 2 */
sky2->tx_le = pci_alloc_consistent(hw->pdev,
TX_RING_SIZE *
@@ -1573,7 +1575,6 @@ static int sky2_down(struct net_device *
/* Stop more packets from being queued */
netif_stop_queue(dev);
- netif_carrier_off(dev);
/* Disable port IRQ */
imask = sky2_read32(hw, B0_IMSK);
@@ -1625,6 +1626,8 @@ static int sky2_down(struct net_device *
sky2_phy_power(hw, port, 0);
+ netif_carrier_off(dev);
+
/* turn off LED's */
sky2_write16(hw, B0_Y2LED, LED_STAT_OFF);
@@ -1689,7 +1692,6 @@ static void sky2_link_up(struct sky2_por
gm_phy_write(hw, port, PHY_MARV_INT_MASK, PHY_M_DEF_MSK);
netif_carrier_on(sky2->netdev);
- netif_wake_queue(sky2->netdev);
/* Turn on link LED */
sky2_write8(hw, SK_REG(port, LNK_LED_REG),
@@ -1741,7 +1743,6 @@ static void sky2_link_down(struct sky2_p
gma_write16(hw, port, GM_GP_CTRL, reg);
netif_carrier_off(sky2->netdev);
- netif_stop_queue(sky2->netdev);
/* Turn on link LED */
sky2_write8(hw, SK_REG(port, LNK_LED_REG), LINKLED_OFF);
@@ -2064,6 +2065,9 @@ static struct sk_buff *sky2_receive(stru
if (!(status & GMR_FS_RX_OK))
goto resubmit;
+ if (status >> 16 != length)
+ goto len_mismatch;
+
if (length < copybreak)
skb = receive_copy(sky2, re, length);
else
@@ -2073,6 +2077,11 @@ resubmit:
return skb;
+len_mismatch:
+ /* Truncation of overlength packets
+ causes PHY length to not match MAC length */
+ ++sky2->net_stats.rx_length_errors;
+
error:
++sky2->net_stats.rx_errors;
if (status & GMR_FS_RX_FF_OV) {
@@ -2441,17 +2450,24 @@ static int sky2_poll(struct net_device *
sky2_phy_intr(hw, 1);
work_done = sky2_status_intr(hw, work_limit);
- if (work_done < work_limit) {
- netif_rx_complete(dev0);
+ *budget -= work_done;
+ dev0->quota -= work_done;
- /* end of interrupt, re-enables also acts as I/O synchronization */
- sky2_read32(hw, B0_Y2_SP_LISR);
- return 0;
- } else {
- *budget -= work_done;
- dev0->quota -= work_done;
+ /* More work? */
+ if (hw->st_idx != sky2_read16(hw, STAT_PUT_IDX))
return 1;
+
+ /* Bug/Errata workaround?
+ * Need to kick the TX irq moderation timer.
+ */
+ if (sky2_read8(hw, STAT_TX_TIMER_CTRL) == TIM_START) {
+ sky2_write8(hw, STAT_TX_TIMER_CTRL, TIM_STOP);
+ sky2_write8(hw, STAT_TX_TIMER_CTRL, TIM_START);
}
+ netif_rx_complete(dev0);
+
+ sky2_read32(hw, B0_Y2_SP_LISR);
+ return 0;
}
static irqreturn_t sky2_intr(int irq, void *dev_id)
@@ -3486,10 +3502,6 @@ static __devinit struct net_device *sky2
memcpy_fromio(dev->dev_addr, hw->regs + B2_MAC_1 + port * 8, ETH_ALEN);
memcpy(dev->perm_addr, dev->dev_addr, dev->addr_len);
- /* device is off until link detection */
- netif_carrier_off(dev);
- netif_stop_queue(dev);
-
return dev;
}
^ permalink raw reply
* Re: [PATCH] atl1: use spin_trylock_irqsave()
From: Jeff Garzik @ 2007-08-07 21:18 UTC (permalink / raw)
To: Jay Cliburn
Cc: mingo, csnook, atl1-devel, netdev, linux-kernel, nando, tglx,
brbrofsvl
In-Reply-To: <11859268222715-git-send-email-jacliburn@bellsouth.net>
Jay Cliburn wrote:
> From: Ingo Molnar <mingo@elte.hu>
>
> use the simpler spin_trylock_irqsave() API to get the adapter lock.
>
> [ this is also a fix for -rt where adapter->lock is a sleeping lock. ]
>
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> Signed-off-by: Jay Cliburn <jacliburn@bellsouth.net>
> ---
> drivers/net/atl1/atl1_main.c | 4 +---
> 1 files changed, 1 insertions(+), 3 deletions(-)
applied to #upstream-fixes
^ permalink raw reply
* Re: [patch] sis190 check for ISA bridge on SiS966
From: Jeff Garzik @ 2007-08-07 21:19 UTC (permalink / raw)
To: maximilian attems; +Cc: netdev, Neil Muller
In-Reply-To: <20070801155204.GB21942@stro.at>
maximilian attems wrote:
> From: Neil Muller <drnlmuller+bugs@gmail.com>
>
> sis190 driver assumes to find ISA only on SiS965.
> similar fix is in sis900 driver, see bug report
> http://bugs.debian.org/435547
>
> Signed-off-by: maximilian attems <max@stro.at>
applied to #upstream-fixes
^ permalink raw reply
* Re: [GIT PATCH] ucc_geth fixes for 2.6.22-rc1
From: Jeff Garzik @ 2007-08-07 21:19 UTC (permalink / raw)
To: leoli; +Cc: netdev, linuxppc-embedded
In-Reply-To: <1186132075.6230.11.camel@Gundam>
Li Yang wrote:
> Please pull from 'ucc_geth' branch of
> master.kernel.org:/pub/scm/linux/kernel/git/leo/fsl-soc.git ucc_geth
>
> to receive the following fixes:
>
> drivers/net/ucc_geth_ethtool.c | 1 -
> drivers/net/ucc_geth_mii.c | 3 ++-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> Domen Puncer (1):
> ucc_geth: fix section mismatch
>
> Jan Altenberg (1):
> ucc_geth: remove get_perm_addr from ucc_geth_ethtool.c
pulled
^ permalink raw reply
* Re: Please pull 'fixes-jgarzik' branch of wireless-2.6
From: Jeff Garzik @ 2007-08-07 21:25 UTC (permalink / raw)
To: John W. Linville
Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20070806201441.GJ6442-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
John W. Linville wrote:
> The following changes since commit d4ac2477fad0f2680e84ec12e387ce67682c5c13:
> Linus Torvalds (1):
> Linux 2.6.23-rc2
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-2.6.git fixes-jgarzik
>
> John W. Linville (1):
> Revert "[PATCH] bcm43xx: Fix deviation from specifications in set_baseband_attenuation"
>
> Masakazu Mokuno (1):
> remove duplicated ioctl entries in compat_ioctl.c
>
> Michael Buesch (1):
> softmac: Fix deadlock of wx_set_essid with assoc work
>
> Michael Wu (1):
> rtl8187: ensure priv->hwaddr is always valid
>
> Ulrich Kunitz (1):
> zd1211rw: fix filter for PSPOLL frames
>
> drivers/net/wireless/bcm43xx/bcm43xx_phy.c | 2 +-
> drivers/net/wireless/rtl8187_dev.c | 2 +-
> drivers/net/wireless/zd1211rw/zd_mac.c | 2 +-
> fs/compat_ioctl.c | 3 ---
> net/ieee80211/softmac/ieee80211softmac_wx.c | 11 ++++++++---
> 5 files changed, 11 insertions(+), 9 deletions(-)
pulled
^ permalink raw reply
* Re: [PATCH 0/2] r8169: pull request for 'r8169-for-jeff-20070806' branch
From: Jeff Garzik @ 2007-08-07 21:28 UTC (permalink / raw)
To: Francois Romieu; +Cc: netdev, akpm
In-Reply-To: <20070806221612.GA23302@electric-eye.fr.zoreil.com>
Francois Romieu wrote:
> Please pull from branch 'r8169-for-jeff-20070806' in repository
>
> git://electric-eye.fr.zoreil.com/home/romieu/linux-2.6.git r8169-for-jeff-20070806
>
> to get the changes below.
>
> Distance from 'upstream-fixes' (c196d80f994ef4ffefd5a7c62e3f42bd75d538bc)
> -------------------------------------------------------------------------
>
> 313b0305b5a1e7e0fb39383befbf79558ce68a9c
> 2584fbc3a61897de5eddd56b39a4fa9cd074eca2
>
> Diffstat
> --------
>
> drivers/net/r8169.c | 24 ++++++++++++++++--------
> 1 files changed, 16 insertions(+), 8 deletions(-)
>
> Shortlog
> --------
>
> Francois Romieu (1):
> r8169: avoid needless NAPI poll scheduling
>
> Roger So (1):
> r8169: PHY power-on fix
pulled
^ permalink raw reply
* Re: [RFT] sky2: backport patch
From: Jeff Garzik @ 2007-08-07 21:33 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Greg KH, Krzysztof Oledzki, netdev, stable
In-Reply-To: <20070807171415.6892a070@oldman>
Stephen Hemminger wrote:
> Any volunteers to test this, it has a backport for the three main stability patches:
> 1. carrier management
> 2. lost irq timer
> 3. rechecking for packets in poll
> 4. overlength packet hang.
>
> I am away from any sky2 hardware for another week, but others maybe
> able to validate this.
Backport to what? from what?
You supplied no kernel version info.
Jeff
^ permalink raw reply
* Re: [RESEND][PATCH 1/3] ehea: Fix workqueue handling
From: Jeff Garzik @ 2007-08-07 21:34 UTC (permalink / raw)
To: Thomas Klein
Cc: Thomas Klein, Jan-Bernd Themann, netdev, linux-kernel,
Christoph Raisch, Stefan Roscher, linux-ppc, Jan-Bernd Themann,
Marcus Eder
In-Reply-To: <200708061354.30653.osstklei@de.ibm.com>
Thomas Klein wrote:
> Fix: Workqueue ehea_driver_wq was not destroyed
>
> Signed-off-by: Thomas Klein <tklein@de.ibm.com>
>
> ---
> drivers/net/ehea/ehea.h | 2 +-
> drivers/net/ehea/ehea_main.c | 1 +
> 2 files changed, 2 insertions(+), 1 deletions(-)
applied 1-3 to #upstream-fixes
^ permalink raw reply
* Re: [PATCH] phy layer: fix phy_mii_ioctl for autonegotiation
From: Jeff Garzik @ 2007-08-07 21:34 UTC (permalink / raw)
To: Domen Puncer; +Cc: netdev, macro
In-Reply-To: <20070807101241.GA13994@moe.telargo.com>
Domen Puncer wrote:
> Fix a thinko (?) in setting phydev->autoneg.
>
>
> Signed-off-by: Domen Puncer <domen.puncer@telargo.com>
>
> ---
> This fixes my "mii.h -> ethtool.h advertising #defines". I'm not sure
> why and how they're translated, but it does work now.
> Maybe they're just ignored, since mii-tool directly reads and writes
> MII registers.
>
>
> drivers/net/phy/phy.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
applied
^ permalink raw reply
* Re: [PATCH] drivers/net/ibmveth.c: memset fix
From: Jeff Garzik @ 2007-08-07 21:36 UTC (permalink / raw)
To: Mariusz Kozlowski; +Cc: brking, santil, netdev, linux-kernel
In-Reply-To: <200708062344.03443.m.kozlowski@tuxland.pl>
Mariusz Kozlowski wrote:
>>>> Looks like memset() is zeroing wrong nr of bytes.
>>> Good catch, however, I think we can just remove this memset altogether
>>> since the memory gets allocated via kzalloc.
>> Correct, that memset() is superfluous.
>
> Ok. Then this should do it.
>
> Signed-off-by: Mariusz Kozlowski <m.kozlowski@tuxland.pl>
>
> drivers/net/ibmveth.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
applied
^ permalink raw reply
* Re: [PATCH 1/6] ibmveth: Enable TCP checksum offload
From: Jeff Garzik @ 2007-08-07 21:46 UTC (permalink / raw)
To: Brian King; +Cc: linuxppc-dev, rcjenn, santil, netdev
In-Reply-To: <11864293333377-patch-mail.ibm.com>
Brian King wrote:
> This patchset enables TCP checksum offload support for IPV4
> on ibmveth. This completely eliminates the generation and checking of
> the checksum for packets that are completely virtual and never
> touch a physical network. A simple TCP_STREAM netperf run on
> a virtual network with maximum mtu set yielded a ~30% increase
> in throughput. This feature is enabled by default on systems that
> support it, but can be disabled with a module option.
>
> Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
ACK, but does not apply to current netdev-2.6.git#upstream-fixes.
Request resend after the ibmveth fixes hit mainline (24 hours or so
after push, I suppose)
^ permalink raw reply
* Re: [RFT] sky2: backport patch
From: Stephen Hemminger @ 2007-08-07 21:47 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Greg KH, Krzysztof Oledzki, netdev, stable
In-Reply-To: <46B8E53F.70605@garzik.org>
On Tue, 07 Aug 2007 17:33:51 -0400
Jeff Garzik <jeff@garzik.org> wrote:
> Stephen Hemminger wrote:
> > Any volunteers to test this, it has a backport for the three main stability patches:
> > 1. carrier management
> > 2. lost irq timer
> > 3. rechecking for packets in poll
> > 4. overlength packet hang.
> >
> > I am away from any sky2 hardware for another week, but others maybe
> > able to validate this.
>
> Backport to what? from what?
>From 2.6.23-rc2 to 2.6.22.y base
> You supplied no kernel version info.
>
> Jeff
>
>
>
^ permalink raw reply
* Re: [PATCH 2/6] ibmveth: Implement ethtool hooks to enable/disable checksum offload
From: Jeff Garzik @ 2007-08-07 21:48 UTC (permalink / raw)
To: Brian King; +Cc: linuxppc-dev, rcjenn, santil, netdev
In-Reply-To: <200708061942.l76JgLrU009242@d01av04.pok.ibm.com>
Brian King wrote:
> This patch adds the appropriate ethtool hooks to allow for enabling/disabling
> of hypervisor assisted checksum offload for TCP.
>
> Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
> ---
>
> linux-2.6-bjking1/drivers/net/ibmveth.c | 118 +++++++++++++++++++++++++++++++-
> linux-2.6-bjking1/drivers/net/ibmveth.h | 1
> 2 files changed, 117 insertions(+), 2 deletions(-)
>
> diff -puN drivers/net/ibmveth.c~ibmveth_csum_offload_ethtool drivers/net/ibmveth.c
> --- linux-2.6/drivers/net/ibmveth.c~ibmveth_csum_offload_ethtool 2007-08-01 14:55:14.000000000 -0500
> +++ linux-2.6-bjking1/drivers/net/ibmveth.c 2007-08-01 14:55:14.000000000 -0500
> @@ -641,12 +641,125 @@ static u32 netdev_get_link(struct net_de
> return 1;
> }
>
> +static void ibmveth_set_rx_csum_flags(struct net_device *dev, u32 data)
> +{
> + struct ibmveth_adapter *adapter = dev->priv;
> +
> + if (data)
> + adapter->rx_csum = 1;
> + else {
> + adapter->rx_csum = 0;
> + dev->features &= ~NETIF_F_IP_CSUM;
why does this RX-related code clear a TX-related flag?
otherwise looks OK
^ permalink raw reply
* Re: [PATCH 3/6] ibmveth: Add ethtool TSO handlers
From: Jeff Garzik @ 2007-08-07 21:50 UTC (permalink / raw)
To: Brian King; +Cc: linuxppc-dev, rcjenn, santil, netdev
In-Reply-To: <200708061942.l76JgSnp016148@d03av04.boulder.ibm.com>
Brian King wrote:
> Add handlers for get_tso and get_ufo to prevent errors being printed
> by ethtool.
>
> Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
> ---
>
> drivers/net/ibmveth.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff -puN drivers/net/ibmveth.c~ibmveth_ethtool_get_tso drivers/net/ibmveth.c
> --- linux-2.6/drivers/net/ibmveth.c~ibmveth_ethtool_get_tso 2007-07-19 11:18:38.000000000 -0500
> +++ linux-2.6-bjking1/drivers/net/ibmveth.c 2007-07-19 11:18:38.000000000 -0500
> @@ -759,7 +759,9 @@ static const struct ethtool_ops netdev_e
> .get_tx_csum = ethtool_op_get_tx_csum,
> .set_tx_csum = ibmveth_set_tx_csum,
> .get_rx_csum = ibmveth_get_rx_csum,
> - .set_rx_csum = ibmveth_set_rx_csum
> + .set_rx_csum = ibmveth_set_rx_csum,
> + .get_tso = ethtool_op_get_tso,
> + .get_ufo = ethtool_op_get_ufo
ACK, once you add a comma to the end of the final initializer
As you see from this patch, the practice of -not- having commas at the
end of a list of struct initializers is not patch-friendly, since you
must touch an unrelated line each time you patch the end of the struct.
For named initializers particularly, the lack of a comma is even more
useless.
So, it might tweak some C perfectionists, but adding that
seemingly-useless comma at the end of the last entry reduces maintenance
headache and makes patch reviews slightly more clear.
Jeff
^ permalink raw reply
* Re: [PATCH 4/6] ibmveth: Add ethtool driver stats hooks
From: Jeff Garzik @ 2007-08-07 21:50 UTC (permalink / raw)
To: Brian King; +Cc: linuxppc-dev, rcjenn, santil, netdev
In-Reply-To: <200708061942.l76JgZRK020999@d03av03.boulder.ibm.com>
Brian King wrote:
> Add ethtool hooks to ibmveth to retrieve driver statistics.
>
> Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
> ---
>
> drivers/net/ibmveth.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 52 insertions(+), 1 deletion(-)
>
> diff -puN drivers/net/ibmveth.c~ibmveth_ethtool_driver_stats drivers/net/ibmveth.c
> --- linux-2.6/drivers/net/ibmveth.c~ibmveth_ethtool_driver_stats 2007-07-19 11:18:41.000000000 -0500
> +++ linux-2.6-bjking1/drivers/net/ibmveth.c 2007-07-19 11:18:41.000000000 -0500
> @@ -112,6 +112,28 @@ MODULE_DESCRIPTION("IBM i/pSeries Virtua
> MODULE_LICENSE("GPL");
> MODULE_VERSION(ibmveth_driver_version);
>
> +struct ibmveth_stat {
> + char name[ETH_GSTRING_LEN];
> + int offset;
> +};
> +
> +#define IBMVETH_STAT_OFF(stat) offsetof(struct ibmveth_adapter, stat)
> +#define IBMVETH_GET_STAT(a, off) *((u64 *)(((unsigned long)(a)) + off))
> +
> +struct ibmveth_stat ibmveth_stats[] = {
> + { "replenish_task_cycles", IBMVETH_STAT_OFF(replenish_task_cycles) },
> + { "replenish_no_mem", IBMVETH_STAT_OFF(replenish_no_mem) },
> + { "replenish_add_buff_failure", IBMVETH_STAT_OFF(replenish_add_buff_failure) },
> + { "replenish_add_buff_success", IBMVETH_STAT_OFF(replenish_add_buff_success) },
> + { "rx_invalid_buffer", IBMVETH_STAT_OFF(rx_invalid_buffer) },
> + { "rx_no_buffer", IBMVETH_STAT_OFF(rx_no_buffer) },
> + { "tx_multidesc_send", IBMVETH_STAT_OFF(tx_multidesc_send) },
> + { "tx_linearized", IBMVETH_STAT_OFF(tx_linearized) },
> + { "tx_linearize_failed", IBMVETH_STAT_OFF(tx_linearize_failed) },
> + { "tx_map_failed", IBMVETH_STAT_OFF(tx_map_failed) },
> + { "tx_send_failed", IBMVETH_STAT_OFF(tx_send_failed) }
> +};
> +
> /* simple methods of getting data from the current rxq entry */
> static inline int ibmveth_rxq_pending_buffer(struct ibmveth_adapter *adapter)
> {
> @@ -751,6 +773,32 @@ static u32 ibmveth_get_rx_csum(struct ne
> return adapter->rx_csum;
> }
>
> +static void ibmveth_get_strings(struct net_device *dev, u32 stringset, u8 *data)
> +{
> + int i;
> +
> + if (stringset != ETH_SS_STATS)
> + return;
> +
> + for (i = 0; i < ARRAY_SIZE(ibmveth_stats); i++, data += ETH_GSTRING_LEN)
> + memcpy(data, ibmveth_stats[i].name, ETH_GSTRING_LEN);
> +}
> +
> +static int ibmveth_get_stats_count(struct net_device *dev)
> +{
> + return ARRAY_SIZE(ibmveth_stats);
> +}
> +
> +static void ibmveth_get_ethtool_stats(struct net_device *dev,
> + struct ethtool_stats *stats, u64 *data)
> +{
> + int i;
> + struct ibmveth_adapter *adapter = dev->priv;
> +
> + for (i = 0; i < ARRAY_SIZE(ibmveth_stats); i++)
> + data[i] = IBMVETH_GET_STAT(adapter, ibmveth_stats[i].offset);
> +}
> +
> static const struct ethtool_ops netdev_ethtool_ops = {
> .get_drvinfo = netdev_get_drvinfo,
> .get_settings = netdev_get_settings,
> @@ -761,7 +809,10 @@ static const struct ethtool_ops netdev_e
> .get_rx_csum = ibmveth_get_rx_csum,
> .set_rx_csum = ibmveth_set_rx_csum,
> .get_tso = ethtool_op_get_tso,
> - .get_ufo = ethtool_op_get_ufo
> + .get_ufo = ethtool_op_get_ufo,
> + .get_strings = ibmveth_get_strings,
> + .get_stats_count = ibmveth_get_stats_count,
> + .get_ethtool_stats = ibmveth_get_ethtool_stats
ACK, modulo comma-at-end-of-initializer per previous email
^ permalink raw reply
* Re: [PATCH 6/6] ibmveth: Remove use of bitfields
From: Jeff Garzik @ 2007-08-07 21:56 UTC (permalink / raw)
To: Brian King; +Cc: linuxppc-dev, rcjenn, santil, netdev
In-Reply-To: <200708061942.l76JgmxI031698@d03av01.boulder.ibm.com>
Brian King wrote:
> Removes the use of bitfields from the ibmveth driver. This results
> in slightly smaller object code.
>
> Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
> ---
>
> linux-2.6-bjking1/drivers/net/ibmveth.c | 90 ++++++++++++++++----------------
> linux-2.6-bjking1/drivers/net/ibmveth.h | 56 ++++++++-----------
> 2 files changed, 68 insertions(+), 78 deletions(-)
strong ACK :)
Though I also encourage you to avoid #defines for named constants, in
favor of
enum {
IBMVETH_BUF_VALID = (1U << 31),
IBMVETH_BUF_TOGGLE = (1U << 30),
IBMVETH_BUF_NO_CSUM = (1U << 25),
IBMVETH_BUF_CSUM_GOOD = (1U << 24),
IBMVETH_BUF_LEN_MASK = 0x00FFFFFF,
};
This illustrates:
1) The "1 << n" notation is FAR easier to read and compare with data
sheets. You're just adding to the trouble by requiring the reviewer's
brain to convert hex numbers to bits, even if most engineers can do this
in their sleep.
2) The named constants are available to the C compiler, which is more
friendly to debuggers. It also supplies type information to the C compiler.
3) Similar to #2, wading through C pre-processor output is much easier
when the symbols don't disappear.
These are recommendations, not requirements, but I've found these
techniques superior to cpp in many other drivers.
Jeff
^ permalink raw reply
* Re: [PATCH 68] drivers/net/s2io.c: kmalloc + memset conversion to k[cz]alloc
From: Jeff Garzik @ 2007-08-07 21:57 UTC (permalink / raw)
To: Mariusz Kozlowski; +Cc: linux-kernel, kernel-janitors, Andrew Morton, netdev
In-Reply-To: <200707312356.58934.m.kozlowski@tuxland.pl>
Mariusz Kozlowski wrote:
> Signed-off-by: Mariusz Kozlowski <m.kozlowski@tuxland.pl>
>
> drivers/net/s2io.c | 235587 -> 235340 (-247 bytes)
> drivers/net/s2io.o | 460768 -> 460120 (-648 bytes)
>
> drivers/net/s2io.c | 14 +++++---------
> 1 file changed, 5 insertions(+), 9 deletions(-)
ACK but didn't apply, please wait 24-48 hours (so that s2io fixes go
upstream), then rediff and resend
^ permalink raw reply
* Re: [PATCH 69] drivers/net/sb1250-mac.c: kmalloc + memset conversion to kcalloc
From: Jeff Garzik @ 2007-08-07 21:57 UTC (permalink / raw)
To: Mariusz Kozlowski; +Cc: linux-kernel, kernel-janitors, Andrew Morton, netdev
In-Reply-To: <200707312358.36567.m.kozlowski@tuxland.pl>
Mariusz Kozlowski wrote:
> Signed-off-by: Mariusz Kozlowski <m.kozlowski@tuxland.pl>
>
> drivers/net/sb1250-mac.c | 76286 -> 76199 (-87 bytes)
>
> drivers/net/sb1250-mac.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
applied
^ 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