* 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 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: [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 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: [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] 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: [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: [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: [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: 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: [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: [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: [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
* [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
* [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
* 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
* 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
* 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
* 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
* 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
* [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
* [RFC] cubic: backoff after slow start
From: Stephen Hemminger @ 2007-08-07 18:37 UTC (permalink / raw)
To: Injong Rhee, Sangtae Ha; +Cc: netdev
CUBIC takes several unnecessary iterations to converge out of slow start. This
is most noticable over a link where the bottleneck queue size is much larger than BDP,
and the sender has to "fill the pipe" in slow start before the first loss. Typical
consumer broadband links seem to have large (up to 2secs) of queue that needs
to get filled before the first loss.
A possible fix is to use a beta of .5 (same as original TCP) when leaving
slow start. Originally, the Linux version didn't do slow start so it probably
never was observed.
--- a/net/ipv4/tcp_cubic.c 2007-08-02 12:16:22.000000000 +0100
+++ b/net/ipv4/tcp_cubic.c 2007-08-03 15:57:12.000000000 +0100
@@ -289,7 +289,11 @@ static u32 bictcp_recalc_ssthresh(struct
ca->loss_cwnd = tp->snd_cwnd;
- return max((tp->snd_cwnd * beta) / BICTCP_BETA_SCALE, 2U);
+ /* Initial backoff when leaving slow start */
+ if (tp->snd_ssthresh == 0x7fffffff)
+ return max(tp->snd_cwnd >> 1U, 2U);
+ else
+ return max((tp->snd_cwnd * beta) / BICTCP_BETA_SCALE, 2U);
}
static u32 bictcp_undo_cwnd(struct sock *sk)
^ permalink raw reply
* Re: Distributed storage.
From: Daniel Phillips @ 2007-08-07 18:24 UTC (permalink / raw)
To: Jens Axboe
Cc: Evgeniy Polyakov, netdev, linux-kernel, linux-fsdevel,
Peter Zijlstra
In-Reply-To: <20070807120523.GX5245@kernel.dk>
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
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
bi_destructor could be combined. I don't see a lot of users of bi_idx,
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.
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 :-)
Regards,
Daniel
^ permalink raw reply
* Re: e100
From: Kok, Auke @ 2007-08-07 18:03 UTC (permalink / raw)
To: ericj; +Cc: Jeff Garzik, NetDev
In-Reply-To: <20070807175942.M75711@ericj.net>
ericj wrote:
> I want to thank everyone who helped with this.
>
> It was proven to be a hardware issue. The board designer had left a GPIO
> pin in an indeterminate state because he was planning to use it later to
> do something with the battery charge circuitry.
>
> I apologize for wasting everyone's time.
happens to everyone :)
Thanks for letting us know.
Auke
>
> On Mon, 06 Aug 2007 17:45:09 -0700, Kok, Auke wrote
>> [moving to netdev mailinglist]
>>
>> ericj wrote:
>>> On Mon, 6 Aug 2007 11:20:58 -0500, ericj wrote
>>>> On Mon, 06 Aug 2007 12:13:28 -0400, Jeff Garzik wrote
>>>>> eepro100 is going to be removed. Please try e100 on 2.6.22 or
>>>>> 2.6.23-rc2.
>>>> I will give the 2.6.23 a try.
>>> I tried 2.6.23-rc2 and there was no change.
>>>
>>> There is now some question from the hardware guys about whether the
>>> eeproms were properly configured before shipping the boards. Is there
>>> any documentation of the eeprom on an EE Pro 100 VE (ICH4) so that I can
>>> figure out if any of the settings in there might be causing the problem?
>>>
>>> The only fields I know of for sure are the MAC address at the beginning
>>> and the checksum at the end. I also see from the driver code that there
>>> is at least one byte controlling wake-on-lan, which I don't care about -
>>> unless it's the problem.
>>>
>>> Thanks for ethtool, by the way. It's been helpful in looking at this and
>>> comparing the eeprom to an earlier version of the board that works.
>> Eric,
>>
>> please don't forget that an entire team here at Intel is
>> dedicated to supporting e100 and pro/1000 devices from Intel.
>>
>> Most of the pro/100 features are documented in the SDM which
>> contains some references to the eeprom parts. Mostly the
>> device doesn't need much configuration from the eeprom to work
>> (unlike gigE parts). The SDM can be downloaded from our sf.net
>> project page:
>>
>>
> http://sourceforge.net/project/showfiles.php?group_id=42302&package_id=68544
>> The issue that you are reporting:
>>
>> "My system boots fine but when I try to bring up the onboard
>> ethernet (an EEPro 100 VE) I get a "Nobody Cares" message and
>> the interrupt is disabled."
>>
>> However has been recently patched. This should have worked
>> regardless of whether you used e100 or eepro100 (noting that
>> nobody supports eepro100 anymore, you should really use e100
>> for all tests).
>>
>> if you look in drivers/pci/quirks.c you'll find that there is
>> specific code for e100 devices. If this quirk doesn't work for
>> you then we'll need to dig into that. For this I'd like you to
>> gather:
>>
>> - `ethtool -e eth0` output
>> - `lspci -n` output
>>
>> this will allow me to check the quirck code and see if it has
>> the right device ID. I'm suspecting that the device ID is
>> missing somehow, or the workaround fails.
>>
>> Auke
>
>
> --
>
> "A hunch is creativity trying to tell you something" -- Frank Capra
>
> Eric Johnson
^ permalink raw reply
* Re: e100 (was: eepro100) - Nobody Cares (hardware?)
From: ericj @ 2007-08-07 18:01 UTC (permalink / raw)
To: Kok, Auke; +Cc: Jeff Garzik, NetDev
In-Reply-To: <46B7C095.5010202@intel.com>
I want to thank everyone who helped with this.
It was proven to be a hardware issue. The board designer had left a GPIO
pin in an indeterminate state because he was planning to use it later to
do something with the battery charge circuitry.
I apologize for wasting everyone's time.
On Mon, 06 Aug 2007 17:45:09 -0700, Kok, Auke wrote
> [moving to netdev mailinglist]
>
> ericj wrote:
> > On Mon, 6 Aug 2007 11:20:58 -0500, ericj wrote
> >> On Mon, 06 Aug 2007 12:13:28 -0400, Jeff Garzik wrote
> >>> eepro100 is going to be removed. Please try e100 on 2.6.22 or
> >>> 2.6.23-rc2.
> >
> >> I will give the 2.6.23 a try.
> >
> > I tried 2.6.23-rc2 and there was no change.
> >
> > There is now some question from the hardware guys about whether the
> > eeproms were properly configured before shipping the boards. Is there
> > any documentation of the eeprom on an EE Pro 100 VE (ICH4) so that I can
> > figure out if any of the settings in there might be causing the problem?
> >
> > The only fields I know of for sure are the MAC address at the beginning
> > and the checksum at the end. I also see from the driver code that there
> > is at least one byte controlling wake-on-lan, which I don't care about -
> > unless it's the problem.
> >
> > Thanks for ethtool, by the way. It's been helpful in looking at this and
> > comparing the eeprom to an earlier version of the board that works.
>
> Eric,
>
> please don't forget that an entire team here at Intel is
> dedicated to supporting e100 and pro/1000 devices from Intel.
>
> Most of the pro/100 features are documented in the SDM which
> contains some references to the eeprom parts. Mostly the
> device doesn't need much configuration from the eeprom to work
> (unlike gigE parts). The SDM can be downloaded from our sf.net
> project page:
>
>
http://sourceforge.net/project/showfiles.php?group_id=42302&package_id=68544
>
> The issue that you are reporting:
>
> "My system boots fine but when I try to bring up the onboard
> ethernet (an EEPro 100 VE) I get a "Nobody Cares" message and
> the interrupt is disabled."
>
> However has been recently patched. This should have worked
> regardless of whether you used e100 or eepro100 (noting that
> nobody supports eepro100 anymore, you should really use e100
> for all tests).
>
> if you look in drivers/pci/quirks.c you'll find that there is
> specific code for e100 devices. If this quirk doesn't work for
> you then we'll need to dig into that. For this I'd like you to
> gather:
>
> - `ethtool -e eth0` output
> - `lspci -n` output
>
> this will allow me to check the quirck code and see if it has
> the right device ID. I'm suspecting that the device ID is
> missing somehow, or the workaround fails.
>
> Auke
--
"A hunch is creativity trying to tell you something" -- Frank Capra
Eric Johnson
^ 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