* Re: occasionally corrupted network stats in /proc/net/dev
From: Mark Seger @ 2008-01-14 20:05 UTC (permalink / raw)
To: Michael Chan; +Cc: Eric Dumazet, Ben Greear, netdev
In-Reply-To: <1200343269.15122.23.camel@dell>
outstanding! I'm just happy to hear it's not a bug in my monitoring
code... 8-)
-mark
Michael Chan wrote:
> On Mon, 2008-01-14 at 20:12 +0100, Eric Dumazet wrote:
>
>> Mark Seger a écrit :
>>
>>> Ignore that last one as it was pointed out to me that we have both nic
>>> installed on many of our systems and ethtool told me the one
>>> associated with the nic is actually the broadcom one.
>>>
>>> version: 1.4.38 E1B1EC867DEEB8027B2DA0F
>>> license: GPL
>>> description: Broadcom NetXtreme II BCM5706/5708 Driver
>>>
>>>
>> I remember some tg3 chips actually have bugs when reporting stats....
>> once in a while
>>
>> CCed to Michael Chan to get some details.
>>
>
> Yes, that's right. Some BNX2 chips have this problem and we have a
> workaround:
>
> http://git.kernel.org/?p=linux/kernel/git/davem/net-2.6.git;a=commit;h=02537b0676930b1bd9aff2139e0e645c79986931
>
> The chip sometimes DMA wrong counter values if the chip is also
> internally gathering the counters at the time of the DMA.
>
> Driver 1.5.11 and later versions have this workaround.
>
>
>
^ permalink raw reply
* Why are network counters only updated once a second?
From: Mark Seger @ 2008-01-14 20:30 UTC (permalink / raw)
To: netdev
I had mentioned this in my previous post but perhaps it might get more
attention all by itself. I can't say for sure when this changed, but
for the longest time network counters were only updated once every
0.9765 seconds and unless you used a tools like collectl that could
monitor at fractional intervals, your traffic was under-reported AND
you'd get periodic spikes of double the actual rate. See
http://collectl.sourceforge.net/NetworkStats.html for a more complete
explanation.
Eventually the frequency became better aligned at a 1 second interval
because now the number look better, but the problem I see is that when
the sampling interval is very close to the monitoring interval you still
get periodic incorrect data. Furthermore, you now need to know which
way the counters are updated before you pick a sampling interval! But
the real point is if anyone ever wants to do finer grained monitoring,
say every 1/2 or even tenth of a second, they can't because the counters
won't change between samples. Has this ever been discussed before?
-mark
^ permalink raw reply
* Re: Why are network counters only updated once a second?
From: Eric Dumazet @ 2008-01-14 20:45 UTC (permalink / raw)
To: Mark Seger; +Cc: netdev
In-Reply-To: <478BC662.6030004@hp.com>
Mark Seger a écrit :
> I had mentioned this in my previous post but perhaps it might get more
> attention all by itself. I can't say for sure when this changed, but
> for the longest time network counters were only updated once every
> 0.9765 seconds and unless you used a tools like collectl that could
> monitor at fractional intervals, your traffic was under-reported AND
> you'd get periodic spikes of double the actual rate. See
> http://collectl.sourceforge.net/NetworkStats.html for a more complete
> explanation.
>
> Eventually the frequency became better aligned at a 1 second interval
> because now the number look better, but the problem I see is that when
> the sampling interval is very close to the monitoring interval you still
> get periodic incorrect data. Furthermore, you now need to know which
> way the counters are updated before you pick a sampling interval! But
> the real point is if anyone ever wants to do finer grained monitoring,
> say every 1/2 or even tenth of a second, they can't because the counters
> won't change between samples. Has this ever been discussed before?
>
Yes it was discussed before. Some devices perform counters updates directly at
the NIC level, and one in a while a transfert of counters is done to the host.
This is supposed to be better, especially on SMP.
Maybe you need to setup accounting rules with iptables, so that you can
perform counter sampling at whatever rate you want ?
^ permalink raw reply
* Re: [Bugme-new] [Bug 9721] New: wake on lan fails with sky2 module
From: Andrew Morton @ 2008-01-14 20:50 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: supersud501, rjw, netdev, linux-acpi, bugme-daemon
In-Reply-To: <20080114083926.03232343@deepthought>
On Mon, 14 Jan 2008 08:39:26 -0800
Stephen Hemminger <shemminger@linux-foundation.org> wrote:
> > diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
> > index c27c7d6..4f41a94 100644
> > --- a/drivers/net/sky2.c
> > +++ b/drivers/net/sky2.c
> > @@ -2791,6 +2791,9 @@ static void sky2_reset(struct sky2_hw *hw)
> > sky2_write8(hw, B0_CTST, CS_RST_SET);
> > sky2_write8(hw, B0_CTST, CS_RST_CLR);
> >
> > + /* allow writes to PCI config */
> > + sky2_write8(hw, B2_TST_CTRL1, TST_CFG_WRITE_ON);
> > +
> > /* clear PCI errors, if any */
> > pci_read_config_word(pdev, PCI_STATUS, &status);
> > status |= PCI_STATUS_ERROR_BITS;
> >
> > fixes this regression?
> >
> > If so, we should revert that change.
> >
> > > but i noticed another "bug" on 2.6.24-rc7-git with sky2: dmesg shows a
> > > lot of lines every 5 seconds:
> > >
> > > [...]
> > > [ 357.400462] sky2 0000:02:00.0: error interrupt status=0xc0000000
> > > [ 362.442039] printk: 41 messages suppressed.
> > > [ 362.442043] sky2 0000:02:00.0: error interrupt status=0x80000000
> > > [ 367.439151] printk: 18 messages suppressed.
> > > [ 367.439156] sky2 0000:02:00.0: error interrupt status=0x80000000
> > > [ 372.436267] printk: 30 messages suppressed.
> > > [ 372.436271] sky2 0000:02:00.0: error interrupt status=0x80000000
> > > [ 377.350236] printk: 19 messages suppressed.
> > > [...]
> > >
> > > since i do not notice any errors (yet) i'll wait till next rc, maybe it
> > > will be gone then...
> >
> > That's not good. is this new behaviour?
> >
>
> No, reverting that change will break other systems (including mine).
Reverting which change? ac93a3946b676025fa55356180e8321639744b31?
Linus has very clearly stated on multiple occasions that patches which
fix machine A and break machine B will be reverted. For good reasons.
I don't have a copy of those reasons handy, but it should be a well-known
thing.
If you're really interested we can cc him for a reminder, but the effects
of that upon ac93a3946b676025fa55356180e8321639744b31 might be quick. And
terminal.
^ permalink raw reply
* Re: [BUG] skge 0000:02:05: read data parity error
From: Stephen Hemminger @ 2008-01-14 20:52 UTC (permalink / raw)
To: Oliver Pinter (Pintér Olivér); +Cc: linux-kernel, netdev, oliver.pntr
In-Reply-To: <6101e8c40801141157j1ce7d3f0if2e4eb2344b6c844@mail.gmail.com>
On Mon, 14 Jan 2008 20:57:49 +0100
"Oliver Pinter (Pintér Olivér)" <oliver.pntr@gmail.com> wrote:
> Hi All!
>
> It is fully reproductable under 2.6.22.15, 2.6.23.13 (all tainted and
> not tainted [4 different kernel] ) and 2 different PC:
>
> [BUG] skge 0000:02:05: read data parity error
> [BUG] skge 0000:02:05: read data parity error
>
> steps:
> 1. login as root
> 2. start mc
> 3. cd /sys/bus/pci/drivers/skge/0000:02:05.0
> 4. press F3 (mcview) on resource0
> 5. the system hang up, without panic or bug ... only this message
> printed 2x: [BUG] skge 0000:02:05: read data parity error
>
This is not a bug.
The hardware has some debug registers that if accessed cause a read
back to the host. Since this can point anywhere, it will cause errors
or system hang.
The point is don't do it.
--
Stephen Hemminger <stephen.hemminger@vyatta.com>
--
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: Why are network counters only updated once a second?
From: Mark Seger @ 2008-01-14 21:00 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <478BC9ED.2000204@cosmosbay.com>
Eric Dumazet wrote:
> Mark Seger a écrit :
>> I had mentioned this in my previous post but perhaps it might get
>> more attention all by itself. I can't say for sure when this
>> changed, but for the longest time network counters were only updated
>> once every 0.9765 seconds and unless you used a tools like collectl
>> that could monitor at fractional intervals, your traffic was
>> under-reported AND you'd get periodic spikes of double the actual
>> rate. See http://collectl.sourceforge.net/NetworkStats.html for a
>> more complete explanation.
>>
>> Eventually the frequency became better aligned at a 1 second interval
>> because now the number look better, but the problem I see is that
>> when the sampling interval is very close to the monitoring interval
>> you still get periodic incorrect data. Furthermore, you now need to
>> know which way the counters are updated before you pick a sampling
>> interval! But the real point is if anyone ever wants to do finer
>> grained monitoring, say every 1/2 or even tenth of a second, they
>> can't because the counters won't change between samples. Has this
>> ever been discussed before?
>>
>
> Yes it was discussed before. Some devices perform counters updates
> directly at the NIC level, and one in a while a transfert of counters
> is done to the host.
>
> This is supposed to be better, especially on SMP.
>
> Maybe you need to setup accounting rules with iptables, so that you
> can perform counter sampling at whatever rate you want ?
Maybe I wasn't clear enough. I'm grabbing the counters from
/proc/net/dev and whatever mechanism is being used only ports them with
a granularity of about once a second. This means any of the standard
tools that use /proc to get their data will all have the same problem.
-mark
^ permalink raw reply
* Re: [BUG] skge 0000:02:05: read data parity error
From: Adrian Bunk @ 2008-01-14 21:01 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Oliver Pinter (Pintér Olivér), linux-kernel, netdev,
gregkh
In-Reply-To: <20080114125200.28cb4c69@deepthought>
On Mon, Jan 14, 2008 at 12:52:00PM -0800, Stephen Hemminger wrote:
> On Mon, 14 Jan 2008 20:57:49 +0100
> "Oliver Pinter (Pintér Olivér)" <oliver.pntr@gmail.com> wrote:
>
> > Hi All!
> >
> > It is fully reproductable under 2.6.22.15, 2.6.23.13 (all tainted and
> > not tainted [4 different kernel] ) and 2 different PC:
> >
> > [BUG] skge 0000:02:05: read data parity error
> > [BUG] skge 0000:02:05: read data parity error
> >
> > steps:
> > 1. login as root
> > 2. start mc
> > 3. cd /sys/bus/pci/drivers/skge/0000:02:05.0
> > 4. press F3 (mcview) on resource0
> > 5. the system hang up, without panic or bug ... only this message
> > printed 2x: [BUG] skge 0000:02:05: read data parity error
>
> This is not a bug.
>
> The hardware has some debug registers that if accessed cause a read
> back to the host. Since this can point anywhere, it will cause errors
> or system hang.
>
> The point is don't do it.
Is it really a good idea that _reading_ files under /sys can kill your
machine?
That sounds like a huge trap for people debugging their machine (or e.g.
forgetting to exclude /sys from their backup).
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
--
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
* [PATCH] [IPV4] fib_trie: size and statistics
From: Stephen Hemminger @ 2008-01-14 20:57 UTC (permalink / raw)
To: David Miller; +Cc: robert.olsson, netdev, stephen.hemminger
In-Reply-To: <20080112.214417.154179770.davem@davemloft.net>
Show number of entries in trie, the size field was being set but never used,
but it only counted leaves, not all entries. Refactor the two cases in
fib_triestat_seq_show into a single routine.
Note: the stat structure was being malloc'd but the stack usage isn't so
high (288 bytes) that it is worth the additional complexity.
Signed-off-by: Stephen Hemminger <stephen.hemminger@vyatta.com>
---
Patch against current net-2.6.25
--- a/net/ipv4/fib_trie.c 2008-01-14 10:16:06.000000000 -0800
+++ b/net/ipv4/fib_trie.c 2008-01-14 10:30:11.000000000 -0800
@@ -148,10 +148,10 @@ struct trie_stat {
struct trie {
struct node *trie;
+ unsigned int size;
#ifdef CONFIG_IP_FIB_TRIE_STATS
struct trie_use_stats stats;
#endif
- int size;
};
static void put_child(struct trie *t, struct tnode *tn, int i, struct node *n);
@@ -1045,7 +1045,6 @@ static struct list_head *fib_insert_node
insert_leaf_info(&l->list, li);
goto done;
}
- t->size++;
l = leaf_new();
if (!l)
@@ -1258,6 +1257,8 @@ static int fn_trie_insert(struct fib_tab
list_add_tail_rcu(&new_fa->fa_list,
(fa ? &fa->fa_list : fa_head));
+ t->size++;
+
rt_cache_flush(-1);
rtmsg_fib(RTM_NEWROUTE, htonl(key), new_fa, plen, tb->tb_id,
&cfg->fc_nlinfo, 0);
@@ -2128,50 +2129,34 @@ static void trie_show_usage(struct seq_f
}
#endif /* CONFIG_IP_FIB_TRIE_STATS */
+static void fib_trie_show(struct seq_file *seq, const char *name, struct trie *trie)
+{
+ struct trie_stat stat;
+
+ seq_printf(seq, "%s: %d\n", name, trie->size);
+ trie_collect_stats(trie, &stat);
+ trie_show_stats(seq, &stat);
+#ifdef CONFIG_IP_FIB_TRIE_STATS
+ trie_show_usage(seq, &trie->stats);
+#endif
+}
static int fib_triestat_seq_show(struct seq_file *seq, void *v)
{
struct net *net = (struct net *)seq->private;
- struct trie *trie_local, *trie_main;
- struct trie_stat *stat;
struct fib_table *tb;
- trie_local = NULL;
+ seq_printf(seq,
+ "Basic info: size of leaf: %Zd bytes, size of tnode: %Zd bytes.\n",
+ sizeof(struct leaf), sizeof(struct tnode));
+
tb = fib_get_table(net, RT_TABLE_LOCAL);
if (tb)
- trie_local = (struct trie *) tb->tb_data;
-
- trie_main = NULL;
+ fib_trie_show(seq, "Local", (struct trie *) tb->tb_data);
+
tb = fib_get_table(net, RT_TABLE_MAIN);
if (tb)
- trie_main = (struct trie *) tb->tb_data;
-
-
- stat = kmalloc(sizeof(*stat), GFP_KERNEL);
- if (!stat)
- return -ENOMEM;
-
- seq_printf(seq, "Basic info: size of leaf: %Zd bytes, size of tnode: %Zd bytes.\n",
- sizeof(struct leaf), sizeof(struct tnode));
-
- if (trie_local) {
- seq_printf(seq, "Local:\n");
- trie_collect_stats(trie_local, stat);
- trie_show_stats(seq, stat);
-#ifdef CONFIG_IP_FIB_TRIE_STATS
- trie_show_usage(seq, &trie_local->stats);
-#endif
- }
-
- if (trie_main) {
- seq_printf(seq, "Main:\n");
- trie_collect_stats(trie_main, stat);
- trie_show_stats(seq, stat);
-#ifdef CONFIG_IP_FIB_TRIE_STATS
- trie_show_usage(seq, &trie_main->stats);
-#endif
- }
- kfree(stat);
+ fib_trie_show(seq, "Main", (struct trie *) tb->tb_data);
return 0;
}
^ permalink raw reply
* Re: [RFT] sky2: wake-on-lan configuration issues
From: supersud501 @ 2008-01-14 21:05 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Rafael J. Wysocki, Andrew Morton, netdev, linux-acpi,
bugme-daemon
In-Reply-To: <20080114101439.72304b92@deepthought>
Stephen Hemminger wrote:
> Please test this patch against Linus's current (approx 2.6.24-rc7-git5).
> Ignore Andrew's premature reversion attempt...
>
> This patch disables config mode access after clearing PCI settings.
>
> Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
>
> --- a/drivers/net/sky2.c 2008-01-14 09:44:22.000000000 -0800
> +++ b/drivers/net/sky2.c 2008-01-14 09:44:51.000000000 -0800
> @@ -621,6 +621,7 @@ static void sky2_phy_power(struct sky2_h
> static const u32 phy_power[] = { PCI_Y2_PHY1_POWD, PCI_Y2_PHY2_POWD };
> static const u32 coma_mode[] = { PCI_Y2_PHY1_COMA, PCI_Y2_PHY2_COMA };
>
> + sky2_write8(hw, B2_TST_CTRL1, TST_CFG_WRITE_ON);
> reg1 = sky2_pci_read32(hw, PCI_DEV_REG1);
> /* Turn on/off phy power saving */
> if (onoff)
> @@ -632,7 +633,8 @@ static void sky2_phy_power(struct sky2_h
> reg1 |= coma_mode[port];
>
> sky2_pci_write32(hw, PCI_DEV_REG1, reg1);
> - reg1 = sky2_pci_read32(hw, PCI_DEV_REG1);
> + sky2_write8(hw, B2_TST_CTRL1, TST_CFG_WRITE_OFF);
> + sky2_pci_read32(hw, PCI_DEV_REG1);
>
> udelay(100);
> }
> @@ -2426,6 +2428,7 @@ static void sky2_hw_intr(struct sky2_hw
> if (status & (Y2_IS_MST_ERR | Y2_IS_IRQ_STAT)) {
> u16 pci_err;
>
> + sky2_write8(hw, B2_TST_CTRL1, TST_CFG_WRITE_ON);
> pci_err = sky2_pci_read16(hw, PCI_STATUS);
> if (net_ratelimit())
> dev_err(&pdev->dev, "PCI hardware error (0x%x)\n",
> @@ -2433,12 +2436,14 @@ static void sky2_hw_intr(struct sky2_hw
>
> sky2_pci_write16(hw, PCI_STATUS,
> pci_err | PCI_STATUS_ERROR_BITS);
> + sky2_write8(hw, B2_TST_CTRL1, TST_CFG_WRITE_OFF);
> }
>
> if (status & Y2_IS_PCI_EXP) {
> /* PCI-Express uncorrectable Error occurred */
> u32 err;
>
> + sky2_write8(hw, B2_TST_CTRL1, TST_CFG_WRITE_ON);
> err = sky2_read32(hw, Y2_CFG_AER + PCI_ERR_UNCOR_STATUS);
> sky2_write32(hw, Y2_CFG_AER + PCI_ERR_UNCOR_STATUS,
> 0xfffffffful);
> @@ -2446,6 +2451,7 @@ static void sky2_hw_intr(struct sky2_hw
> dev_err(&pdev->dev, "PCI Express error (0x%x)\n", err);
>
> sky2_read32(hw, Y2_CFG_AER + PCI_ERR_UNCOR_STATUS);
> + sky2_write8(hw, B2_TST_CTRL1, TST_CFG_WRITE_OFF);
> }
>
> if (status & Y2_HWE_L1_MASK)
> @@ -2811,6 +2817,7 @@ static void sky2_reset(struct sky2_hw *h
> }
>
> sky2_power_on(hw);
> + sky2_write8(hw, B2_TST_CTRL1, TST_CFG_WRITE_OFF);
>
> for (i = 0; i < hw->ports; i++) {
> sky2_write8(hw, SK_REG(i, GMAC_LINK_CTRL), GMLC_RST_SET);
>
yes, that did it! just tested it (current linus git tree with patched
sky with above patch), everything is clean now (dmesg output) and wol
works even with the commit ac93a3946b676025fa55356180e8321639744b31
so it the bug is fixed without the need to revert
ac93a3946b676025fa55356180e8321639744b31.
^ permalink raw reply
* Re: [PATCH 1/3] drivers/misc :UCC based TDM driver for MPC83xx platforms.
From: Andrew Morton @ 2008-01-14 21:14 UTC (permalink / raw)
To: Poonam_Aggrwal-b10812
Cc: rubini, linux-kernel, linuxppc-dev, netdev, kumar.gala,
michael.barkowski, kim.phillips, ashish.kalra, rich.cutler
In-Reply-To: <Pine.LNX.4.64.0712101710270.29623@linux121>
On Mon, 10 Dec 2007 17:34:44 +0530 (IST)
Poonam_Aggrwal-b10812 <b10812@freescale.com> wrote:
> From: Poonam Aggrwal <b10812@freescale.com>
>
> The UCC TDM driver basically multiplexes and demultiplexes data from
> different channels. It can interface with for example SLIC kind of devices
> to receive TDM data demultiplex it and send to upper applications. At the
> transmit end it receives data for different channels multiplexes it and
> sends them on the TDM channel. It internally uses TSA( Time Slot Assigner)
> which does multiplexing and demultiplexing, UCC to perform SDMA between
> host buffers and the TSA, CMX to connect TSA to UCC.
>
> This driver will run on MPC8323E-RDB platforms.
>
> ...
>
> +#define PREV_PHASE(x) ((x == 0) ? MAX_PHASE : (x - 1))
> +#define NEXT_PHASE(x) (((x + 1) > MAX_PHASE) ? 0 : (x + 1))
These macros can reference their arg more than once and are hence
dangerous. What does PREV_PHASE(foo++) do to foo?
And, in general: do not implement in cpp that which could have been
implemented in C.
> +static struct ucc_tdm_info utdm_primary_info = {
> + .uf_info = {
> + .tsa = 1,
> + .cdp = 1,
> + .cds = 1,
> + .ctsp = 1,
> + .ctss = 1,
> + .revd = 1,
> + .urfs = 0x128,
> + .utfs = 0x128,
> + .utfet = 0,
> + .utftt = 0x128,
> + .ufpt = 256,
> + .ttx_trx = UCC_FAST_GUMR_TRANSPARENT_TTX_TRX_TRANSPARENT,
> + .tenc = UCC_FAST_TX_ENCODING_NRZ,
> + .renc = UCC_FAST_RX_ENCODING_NRZ,
> + .tcrc = UCC_FAST_16_BIT_CRC,
> + .synl = UCC_FAST_SYNC_LEN_NOT_USED,
> + },
> + .ucc_busy = 0,
> +};
> +
> +static struct ucc_tdm_info utdm_info[8];
> +
> +static void dump_siram(struct tdm_ctrl *tdm_c)
> +{
> +#if defined(DEBUG)
Microscopic note: kernel code tends to do
#ifdef FOO
if only one identifier is being tested and
#if defined(FOO) && defined(BAR)
if more than one is being tested.
There is no rational reason for this ;)
> + int i;
> + u16 phy_num_ts;
> +
> + phy_num_ts = tdm_c->physical_num_ts;
> +
> + pr_debug("SI TxRAM dump\n");
> + /* each slot entry in SI RAM is of 2 bytes */
> + for (i = 0; i < phy_num_ts * 2; i++)
> + pr_debug("%x ", in_8(&qe_immr->sir.tx[i]));
> + pr_debug("\nSI RxRAM dump\n");
> + for (i = 0; i < phy_num_ts * 2; i++)
> + pr_debug("%x ", in_8(&qe_immr->sir.rx[i]));
> + pr_debug("\n");
> +#endif
> +}
> +
> +/*
> + * converts u-law compressed samples to linear PCM
> + * If the CONFIG_TDM_LINEAR_PCM flag is not set the
> + * TDM driver receives u-law compressed data from the
> + * SLIC device. This function converts the compressed
> + * data to linear PCM and sends it to upper layers.
> + */
> +static inline int ulaw2int(unsigned char log)
> +{
> + u32 sign, segment, temp, quant;
> + int val;
> +
> + temp = log ^ 0xFF;
> + sign = (temp & 0x80) >> 7;
> + segment = (temp & 0x70) >> 4;
> + quant = temp & 0x0F;
> + quant <<= 1;
> + quant += 33;
> + quant <<= segment;
> + if (sign)
> + val = 33 - quant;
> + else
> + val = quant - 33;
> +
> + val *= 4;
> + return val;
> +}
> +
> +/*
> + * converts linear PCM samples to u-law compressed format.
> + * If the CONFIG_TDM_LINEAR_PCM flag is not set the
> + * TDM driver calls this function to convert the PCM samples
> + * to u-law compressed format before sending them to SLIC
> + * device.
> + */
> +static inline u8 int2ulaw(short linear)
> +{
> + u8 quant, ret;
> + u16 output, absol, temp;
> + u32 i, sign;
> + char segment;
> +
> + ret = 0;
> + if (linear >= 0)
> + linear = (linear >> 2);
> + else
> + linear = (0xc000 | (linear >> 2));
> +
> + absol = abs(linear) + 33;
> + temp = absol;
> + sign = (linear >= 0) ? 1 : 0;
> + for (i = 0; i < 16; i++) {
> + output = temp & 0x8000;
> + if (output)
> + break;
> + temp <<= 1;
> + }
> + segment = 11 - i;
> + quant = (absol >> segment) & 0x0F;
> + segment--;
> + segment <<= 4;
> + output = segment + quant;
> + if (absol > 8191)
> + output = 0x7F;
> + if (sign)
> + ret ^= 0xFF;
> + else
> + ret ^= 0x7F;
> + return ret;
> +}
hrm, how many copies of ulaw/alaw conversion functions do we need in the
tree before someone writes a library function for it?
> + out_be16(&rx_bd->status, bd_status);
> + out_be32(&rx_bd->buf,
> + tdm_c->dma_input_addr + i * SAMPLE_DEPTH * act_num_ts);
> +
> + bd_status = (u16) ((T_R | T_CM | T_W) >> 16);
> + bd_len = SAMPLE_DEPTH * act_num_ts;
> + out_be16(&tx_bd->length, bd_len);
> + out_be16(&tx_bd->status, bd_status);
> + out_be32(&tx_bd->buf,
> + tdm_c->dma_output_addr + i * SAMPLE_DEPTH * act_num_ts);
> +
> + config_si(tdm_c);
> +
> + setbits32(&qe_immr->ic.qimr, (0x80000000 >> ucc));
The compiler treats 0xNNN constants as unsigned so this works OK. I'd have
put a UL on the end of the constant to be sure ;)
> +static int tdm_start(struct tdm_ctrl *tdm_c)
> +{
> + if (request_irq(tdm_c->ut_info->uf_info.irq, tdm_isr,
> + 0, "tdm", tdm_c)) {
> + printk(KERN_ERR "%s: request_irq for tdm_isr failed\n",
> + __FUNCTION__);
> + return -ENODEV;
> + }
> +
> + ucc_fast_enable(tdm_c->uf_private, COMM_DIR_RX | COMM_DIR_TX);
> +
> +#if !defined(CONFIG_TDM_LINEAR_PCM)
> + pr_info("%s 8-bit u-law compressed mode active\n", __FUNCTION__);
> +#else
> + pr_info("%s 16-bit linear pcm mode active with"
> + " slots 0 & 2\n", __FUNCTION__);
> +#endif
Is this the sort of thing which should be controlled at compile-time? I'd
have thought that a runtime control would be more appropriate (a sysfs knob
or a module parameter). Or just work it out automagically?
> + dump_siram(tdm_c);
> + dump_ucc(tdm_c);
> +
> + setbits8(&(qe_immr->si1.siglmr1_h), (0x1 << tdm_c->tdm_port));
> + pr_info("%s UCC based TDM enabled\n", __FUNCTION__);
> +
> + return 0;
> +}
>
> ...
>
> +static void tdm_read(u32 driver_handle, short chn_id, short *pcm_buffer,
> + short len)
> +{
> + int i;
> + u32 phase_rx;
> + /* point to where to start for the current phase data processing */
> + u32 temp_rx;
> +
> + struct tdm_ctrl *tdm_c = (struct tdm_ctrl *)(driver_handle);
eek. What are we doing here, casting a 32-bit quantity to a kernel pointer?
a) Seems to rule out ever using this driver on a 64-bit system
b) It's generally suspicious and indicates that some rethinking is needed.
> +#if !defined(CONFIG_TDM_LINEAR_PCM)
> + u8 *input_tdm_buffer = tdm_c->tdm_input_data;
> +
> +#else
> + u16 *input_tdm_buffer =
> + (u16 *)tdm_c->tdm_input_data;
> +
> +#endif
> + phase_rx = tdm_c->phase_rx;
> + phase_rx = PREV_PHASE(phase_rx);
> +
> + temp_rx = phase_rx * SAMPLE_DEPTH * EFF_ACTIVE_CH;
> +
> +#if defined(UCC_CACHE_SNOOPING_DISABLED)
> + flush_dcache_range((size_t) &input_tdm_buffer[temp_rx],
> + (size_t) &input_tdm_buffer[temp_rx +
> + SAMPLE_DEPTH * ACTIVE_CH]);
> +#endif
Again, is it appropriate that this behaviour be determined at compile-time?
This is very user- and packager- and distributor-unfriendly.
> + for (i = 0; i < len; i++) {
> +#if !defined(CONFIG_TDM_LINEAR_PCM)
> + pcm_buffer[i] =
> + ulaw2int(input_tdm_buffer[i * EFF_ACTIVE_CH +
> + temp_rx + chn_id]);
> +#else
> + pcm_buffer[i] =
> + input_tdm_buffer[i * EFF_ACTIVE_CH + temp_rx + chn_id];
> +#endif
> +
> + }
> +
> +}
> +
> +static int ucc_tdm_probe(struct of_device *ofdev,
> + const struct of_device_id *match)
> +{
> + struct device_node *np = ofdev->node;
> + struct resource res;
> + const unsigned int *prop;
> + u32 ucc_num, device_num, err, ret = 0;
> + struct device_node *np_tmp = NULL;
> + dma_addr_t physaddr;
> + void *tdm_buff;
> + struct ucc_tdm_info *ut_info;
> +
> + prop = of_get_property(np, "device-id", NULL);
> + ucc_num = *prop - 1;
> + if ((ucc_num < 0) || (ucc_num > 7))
> + return -ENODEV;
> +
> + ut_info = &utdm_info[ucc_num];
> + if (ut_info == NULL) {
> + printk(KERN_ERR "additional data missing\n");
> + return -ENODEV;
> + }
> + if (ut_info->ucc_busy) {
> + printk(KERN_ERR "UCC in use by another TDM driver instance\n");
> + return -EBUSY;
> + }
> +
> + ut_info->ucc_busy = 1;
> + tdm_ctrl[num_tdm_devices++] =
> + kzalloc(sizeof(struct tdm_ctrl), GFP_KERNEL);
Shouldn't this check for (num_tdm_devices > MAX_NUM_TDM_DEVICES))?
> + if (!tdm_ctrl[num_tdm_devices - 1]) {
> + printk(KERN_ERR "%s: no memory to allocate for"
> + " tdm control structure\n", __FUNCTION__);
> + num_tdm_devices--;
> + return -ENOMEM;
> + }
> + device_num = num_tdm_devices - 1;
> +
> + tdm_ctrl[device_num]->device = &ofdev->dev;
> + tdm_ctrl[device_num]->ut_info = ut_info;
> +
> + tdm_ctrl[device_num]->ut_info->uf_info.ucc_num = ucc_num;
> +
> + prop = of_get_property(np, "fsl,tdm-num", NULL);
> + if (prop == NULL) {
> + ret = -EINVAL;
> + goto get_property_error;
> + }
>
> ...
>
> +
> +#define SET_RX_SI_RAM(n, val) \
> + out_be16((u16 *)&qe_immr->sir.rx[(n)*2], (u16)(val))
> +
> +#define SET_TX_SI_RAM(n, val) \
> + out_be16((u16 *)&qe_immr->sir.tx[(n)*2], (u16)(val))
I don't think there's anything which requires that these be imlemented in
the preprocessor?
> +struct tdm_cfg {
> + u8 com_pin; /* Common receive and transmit pins
> + * 0 = separate pins
> + * 1 = common pins
> + */
> +
> + u8 fr_sync_level; /* SLx bit Frame Sync Polarity
> + * 0 = L1R/TSYNC active logic "1"
> + * 1 = L1R/TSYNC active logic "0"
> + */
> +
> + u8 clk_edge; /* CEx bit Tx Rx Clock Edge
> + * 0 = TX data on rising edge of clock
> + * RX data on falling edge
> + * 1 = TX data on falling edge of clock
> + * RX data on rising edge
> + */
> +
> + u8 fr_sync_edge; /* FEx bit Frame sync edge
> + * Determine when the sync pulses are sampled
> + * 0 = Falling edge
> + * 1 = Rising edge
> + */
> +
> + u8 rx_fr_sync_delay; /* TFSDx/RFSDx bits Frame Sync Delay
> + * 00 = no bit delay
> + * 01 = 1 bit delay
> + * 10 = 2 bit delay
> + * 11 = 3 bit delay
> + */
> +
> + u8 tx_fr_sync_delay; /* TFSDx/RFSDx bits Frame Sync Delay
> + * 00 = no bit delay
> + * 01 = 1 bit delay
> + * 10 = 2 bit delay
> + * 11 = 3 bit delay
> + */
> +
> + u8 active_num_ts; /* Number of active time slots in TDM
> + * assume same active Rx/Tx time slots
> + */
> +};
Nice commenting.
^ permalink raw reply
* Re: occasionally corrupted network stats in /proc/net/dev
From: Eric Dumazet @ 2008-01-14 17:38 UTC (permalink / raw)
To: Mark Seger; +Cc: netdev
In-Reply-To: <478B99E6.2050800@hp.com>
Mark Seger a écrit :
> I had posted the following on linux-net and haven't see any responses
> possibly because nobody had any or that list is obsolete. I have been
> told this is the current list for everything networking on linux so I
> thought I'd try again...
>
> I suspect the answer will be that it is what it is, but here's the
> deal. I have a tool I use for monitoring network traffic among other
> things - see http://collectl.sourceforge.net/ - and one of its
> benefits is that you can run it continuously as a daemon (similar to
> sar) and generate data in a format suitable for plotting. This means
> that you can automate your entire network monitoring infrastructure at
> fairly fine granularity, down to second if you like. Actually
> 1-second level monitoring will provide incorrect data on earlier
> kernels because the stats aren't updated on 1 second boundaries and
> you need to monitor at an interval of 0.9765 seconds, but that's a
> different story which is explained at
> http://collectl.sourceforge.net/NetworkStats.html
>
> But more importantly, I've found that occasionally (not that often)
> there is bogus data reported from /proc/net/dev. While I don't have a
> lot of details on this it seems to only show up in 64 bit kernels.
> Look at the following samples taken at 1 second intervals:
>
> eth0:135115809 1024897 0 0 0 0 0 9
> 135458926 910340 0 0 0 0 0 0
> eth0:135118023 1024923 0 0 0 0 0 9
> 135460952 910363 0 0 0 0 0 0
> eth0: 0 884620 0 0 0 0 0 909397
> 9687563 1049736 0 0 0 0 0 0
> eth0:135121189 1024957 0 0 0 0 0 9
> 135464222 910400 0 0 0 0 0 0
> eth0:135129565 1024995 0 0 0 0 0 9
> 135473687 910435 0 0 0 0 0 0
>
> see the middle sample? When I look at the change between samples it
> generates a really big number since the difference is assumed to be
> caused a counter wrapping. The problem is it's not always
> straightforward when there is bad data. For example if the original
> and bogus values are close enough it's not even clear there is a problem.
>
> So the obvious question is, is there any way to prevent the bogus data
> from getting reported? If not, is there any way to set the values to
> something to indicate that the correct values can't be determined?
> Clearly this problem would be visible to any tool that looks at /proc
> but since many tools are not automated or don't take it to the level I
> do, nobody probably notices. As for the counter update frequency,
> even though they now appear to be updated closer to a 1 second
> boundary it also means tools that can monitor at sub-second intervals
> will report incorrect data since the counters only change once a second.
What is the NIC used for eth0 (and driver name)
Which version of linux kernel do you run ?
^ permalink raw reply
* Re: [PATCH 0/3] UCC TDM driver for MPC83xx platforms
From: Andrew Morton @ 2008-01-14 21:15 UTC (permalink / raw)
To: Kim Phillips
Cc: Poonam.Aggrwal, sfr, rubini, linux-ppcdev, netdev, linux-kernel,
kumar.gala, Michael.Barkowski, ashish.kalra, Rich.Cutler
In-Reply-To: <20080114120051.3207043b.kim.phillips@freescale.com>
On Mon, 14 Jan 2008 12:00:51 -0600
Kim Phillips <kim.phillips@freescale.com> wrote:
> On Thu, 10 Jan 2008 21:41:20 -0700
> "Aggrwal Poonam" <Poonam.Aggrwal@freescale.com> wrote:
>
> > Hello All
> >
> > I am waiting for more feedback on the patches.
> >
> > If there are no objections please consider them for 2.6.25.
> >
> if this isn't going to go through Alessandro Rubini/misc drivers, can
> it go through the akpm/mm tree?
>
That would work. But it might be more appropriate to go Kumar->paulus->Linus.
^ permalink raw reply
* Re: [BUG] skge 0000:02:05: read data parity error
From: Greg KH @ 2008-01-14 21:30 UTC (permalink / raw)
To: Adrian Bunk
Cc: Stephen Hemminger, Oliver Pinter (Pint??r Oliv??r), linux-kernel,
netdev
In-Reply-To: <20080114210105.GG9847@does.not.exist>
On Mon, Jan 14, 2008 at 11:01:05PM +0200, Adrian Bunk wrote:
> On Mon, Jan 14, 2008 at 12:52:00PM -0800, Stephen Hemminger wrote:
> > On Mon, 14 Jan 2008 20:57:49 +0100
> > "Oliver Pinter (Pint??r Oliv??r)" <oliver.pntr@gmail.com> wrote:
> >
> > > Hi All!
> > >
> > > It is fully reproductable under 2.6.22.15, 2.6.23.13 (all tainted and
> > > not tainted [4 different kernel] ) and 2 different PC:
> > >
> > > [BUG] skge 0000:02:05: read data parity error
> > > [BUG] skge 0000:02:05: read data parity error
> > >
> > > steps:
> > > 1. login as root
> > > 2. start mc
> > > 3. cd /sys/bus/pci/drivers/skge/0000:02:05.0
> > > 4. press F3 (mcview) on resource0
> > > 5. the system hang up, without panic or bug ... only this message
> > > printed 2x: [BUG] skge 0000:02:05: read data parity error
> >
> > This is not a bug.
> >
> > The hardware has some debug registers that if accessed cause a read
> > back to the host. Since this can point anywhere, it will cause errors
> > or system hang.
> >
> > The point is don't do it.
>
> Is it really a good idea that _reading_ files under /sys can kill your
> machine?
No, but mmapping them as root and then doing bad things with that data
isn't recommended :)
Note that this is a special file, just like the ones in /proc, that
provide a mmap interface into the pci card's resource. This is nothing
new...
thanks,
greg k-h
^ permalink raw reply
* Re: [BUG] skge 0000:02:05: read data parity error
From: Oliver Pinter (Pintér Olivér) @ 2008-01-14 21:31 UTC (permalink / raw)
To: Adrian Bunk; +Cc: Stephen Hemminger, linux-kernel, netdev, gregkh
In-Reply-To: <20080114210105.GG9847@does.not.exist>
I think, it is a potential security breakpoint, when applications with
root permission its read, then a machine is freezed, or only i thin
it's?
--
Thanks,
Oliver
^ permalink raw reply
* RE: [PATCH] s2io LRO bugs
From: Ramkrishna Vepa @ 2008-01-14 21:33 UTC (permalink / raw)
To: Al Viro, jgarzik; +Cc: netdev
In-Reply-To: <20071224061435.GT8181@ftp.linux.org.uk>
Al,
Thanks for finding this. We have a few patches lined up and will submit
this patch.
Ram
> -----Original Message-----
> From: netdev-owner@vger.kernel.org
[mailto:netdev-owner@vger.kernel.org]
> On Behalf Of Al Viro
> Sent: Sunday, December 23, 2007 10:15 PM
> To: jgarzik@pobox.com
> Cc: netdev@vger.kernel.org; Ravinandan.Arakali@neterion.com
> Subject: [PATCH] s2io LRO bugs
>
> a) initiate_new_session() sets ->tcp_ack to ntohl(...); everything
> else stores and expects to find there the net-endian value.
> b) check for monotonic timestamps in verify_l3_l4_lro_capable()
> compares the value sitting in TCP option (right there in the
skb->data,
> net-endian 32bit) with the value picked from earlier packet.
> Doing that without ntohl() is an interesting idea and it might even
> work occasionally; unfortunately, it's quite broken.
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
> drivers/net/s2io.c | 20 ++++++++++----------
> drivers/net/s2io.h | 2 +-
> 2 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/s2io.c b/drivers/net/s2io.c
> index 9d80f1c..aef0875 100644
> --- a/drivers/net/s2io.c
> +++ b/drivers/net/s2io.c
> @@ -7898,7 +7898,7 @@ static void initiate_new_session(struct lro
*lro, u8
> *l2h,
> lro->iph = ip;
> lro->tcph = tcp;
> lro->tcp_next_seq = tcp_pyld_len + ntohl(tcp->seq);
> - lro->tcp_ack = ntohl(tcp->ack_seq);
> + lro->tcp_ack = tcp->ack_seq;
> lro->sg_num = 1;
> lro->total_len = ntohs(ip->tot_len);
> lro->frags_len = 0;
> @@ -7907,10 +7907,10 @@ static void initiate_new_session(struct lro
*lro,
> u8 *l2h,
> * already been done.
> */
> if (tcp->doff == 8) {
> - u32 *ptr;
> - ptr = (u32 *)(tcp+1);
> + __be32 *ptr;
> + ptr = (__be32 *)(tcp+1);
> lro->saw_ts = 1;
> - lro->cur_tsval = *(ptr+1);
> + lro->cur_tsval = ntohl(*(ptr+1));
> lro->cur_tsecr = *(ptr+2);
> }
> lro->in_use = 1;
> @@ -7936,7 +7936,7 @@ static void update_L3L4_header(struct s2io_nic
*sp,
> struct lro *lro)
>
> /* Update tsecr field if this session has timestamps enabled */
> if (lro->saw_ts) {
> - u32 *ptr = (u32 *)(tcp + 1);
> + __be32 *ptr = (__be32 *)(tcp + 1);
> *(ptr+2) = lro->cur_tsecr;
> }
>
> @@ -7961,10 +7961,10 @@ static void aggregate_new_rx(struct lro *lro,
> struct iphdr *ip,
> lro->window = tcp->window;
>
> if (lro->saw_ts) {
> - u32 *ptr;
> + __be32 *ptr;
> /* Update tsecr and tsval from this packet */
> - ptr = (u32 *) (tcp + 1);
> - lro->cur_tsval = *(ptr + 1);
> + ptr = (__be32 *) (tcp + 1);
> + lro->cur_tsval = ntohl(*(ptr + 1));
> lro->cur_tsecr = *(ptr + 2);
> }
> }
> @@ -8015,11 +8015,11 @@ static int verify_l3_l4_lro_capable(struct lro
> *l_lro, struct iphdr *ip,
>
> /* Ensure timestamp value increases monotonically */
> if (l_lro)
> - if (l_lro->cur_tsval > *((u32 *)(ptr+2)))
> + if (l_lro->cur_tsval > ntohl(*((__be32
*)(ptr+2))))
> return -1;
>
> /* timestamp echo reply should be non-zero */
> - if (*((u32 *)(ptr+6)) == 0)
> + if (*((__be32 *)(ptr+6)) == 0)
> return -1;
> }
>
> diff --git a/drivers/net/s2io.h b/drivers/net/s2io.h
> index cc1797a..899d60c 100644
> --- a/drivers/net/s2io.h
> +++ b/drivers/net/s2io.h
> @@ -797,7 +797,7 @@ struct lro {
> int in_use;
> __be16 window;
> u32 cur_tsval;
> - u32 cur_tsecr;
> + __be32 cur_tsecr;
> u8 saw_ts;
> };
>
> --
> 1.5.3.GIT
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [BUG] skge 0000:02:05: read data parity error
From: Adrian Bunk @ 2008-01-14 21:34 UTC (permalink / raw)
To: Oliver Pinter (Pintér Olivér)
Cc: Stephen Hemminger, linux-kernel, netdev, gregkh
In-Reply-To: <6101e8c40801141331n3cb66c04pc74ff07faf04c8fd@mail.gmail.com>
On Mon, Jan 14, 2008 at 10:31:03PM +0100, Oliver Pinter (Pintér Olivér) wrote:
> I think, it is a potential security breakpoint, when applications with
> root permission its read, then a machine is freezed, or only i thin
> it's?
When you are root there are infinite ways to kill your machine, so
there's nothing security related about this issue.
> Thanks,
> Oliver
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
^ permalink raw reply
* Re: [BUG] skge 0000:02:05: read data parity error
From: Greg KH @ 2008-01-14 21:38 UTC (permalink / raw)
To: Oliver Pinter (Pint?r Oliv?r)
Cc: Adrian Bunk, Stephen Hemminger, linux-kernel, netdev
In-Reply-To: <6101e8c40801141331n3cb66c04pc74ff07faf04c8fd@mail.gmail.com>
On Mon, Jan 14, 2008 at 10:31:03PM +0100, Oliver Pinter (Pint?r Oliv?r) wrote:
> I think, it is a potential security breakpoint, when applications with
> root permission its read, then a machine is freezed, or only i thin
> it's?
I'm sorry, I don't quite understand what you are trying to say here.
^ permalink raw reply
* Re: [BUG] skge 0000:02:05: read data parity error
From: Oliver Pinter (Pintér Olivér) @ 2008-01-14 21:41 UTC (permalink / raw)
To: Adrian Bunk; +Cc: Stephen Hemminger, linux-kernel, netdev, gregkh
In-Reply-To: <20080114213453.GJ9847@does.not.exist>
On 1/14/08, Adrian Bunk <bunk@kernel.org> wrote:
> On Mon, Jan 14, 2008 at 10:31:03PM +0100, Oliver Pinter (Pintér Olivér)
> wrote:
> > I think, it is a potential security breakpoint, when applications with
> > root permission its read, then a machine is freezed, or only i thin
> > it's?
>
> When you are root there are infinite ways to kill your machine, so
> there's nothing security related about this issue.
Yes, i know, but when some application or daemons read some file with
running root privileges, then ...
thanks, then it is only a "feature" and not bug.
>
> > Thanks,
> > Oliver
>
> cu
> Adrian
>
> --
>
> "Is there not promise of rain?" Ling Tan asked suddenly out
> of the darkness. There had been need of rain for many days.
> "Only a promise," Lao Er said.
> Pearl S. Buck - Dragon Seed
>
>
--
Thanks,
Oliver
^ permalink raw reply
* Re: [BUG] skge 0000:02:05: read data parity error
From: Oliver Pinter (Pintér Olivér) @ 2008-01-14 21:44 UTC (permalink / raw)
To: Greg KH; +Cc: Adrian Bunk, Stephen Hemminger, linux-kernel, netdev
In-Reply-To: <20080114213802.GA7860@suse.de>
On 1/14/08, Greg KH <gregkh@suse.de> wrote:
> On Mon, Jan 14, 2008 at 10:31:03PM +0100, Oliver Pinter (Pint?r Oliv?r)
> wrote:
> > I think, it is a potential security breakpoint, when applications with
> > root permission its read, then a machine is freezed, or only i thinK
> > it's?
>
> I'm sorry, I don't quite understand what you are trying to say here.
>
huh, sorry, it's typo and i'm not learn english, only myself ... and
my spelling is very bad, sorry
--
Thanks,
Oliver
^ permalink raw reply
* Re: Why are network counters only updated once a second?
From: Michael Chan @ 2008-01-14 22:31 UTC (permalink / raw)
To: Mark Seger; +Cc: netdev
In-Reply-To: <478BC662.6030004@hp.com>
On Mon, 2008-01-14 at 15:30 -0500, Mark Seger wrote:
> Eventually the frequency became better aligned at a 1 second interval
> because now the number look better, but the problem I see is that when
> the sampling interval is very close to the monitoring interval you still
> get periodic incorrect data. Furthermore, you now need to know which
> way the counters are updated before you pick a sampling interval! But
> the real point is if anyone ever wants to do finer grained monitoring,
> say every 1/2 or even tenth of a second, they can't because the counters
> won't change between samples. Has this ever been discussed before?
On most Broadcom NICs, the statistics counters are periodically DMA'ed
from the chip and the default interval is roughly 1 second. On most of
these chips, you can override this interval using ethtool -C eth0.
As I mentioned earlier, the 5706/5708 has a bug in the statistics DMA
engine that can corrupt counters from time to time. The workaround is
effective, but you now lose the ability to control the DMA interval.
^ permalink raw reply
* Re: [BUG] skge 0000:02:05: read data parity error
From: Greg KH @ 2008-01-14 21:48 UTC (permalink / raw)
To: Oliver Pinter (Pint?r Oliv?r)
Cc: Adrian Bunk, Stephen Hemminger, linux-kernel, netdev
In-Reply-To: <6101e8c40801141341hcb17b08h2fd5244bccb21928@mail.gmail.com>
On Mon, Jan 14, 2008 at 10:41:52PM +0100, Oliver Pinter (Pint?r Oliv?r) wrote:
> On 1/14/08, Adrian Bunk <bunk@kernel.org> wrote:
> > On Mon, Jan 14, 2008 at 10:31:03PM +0100, Oliver Pinter (Pint?r Oliv?r)
> > wrote:
> > > I think, it is a potential security breakpoint, when applications with
> > > root permission its read, then a machine is freezed, or only i thin
> > > it's?
> >
> > When you are root there are infinite ways to kill your machine, so
> > there's nothing security related about this issue.
>
> Yes, i know, but when some application or daemons read some file with
> running root privileges, then ...
It's always been that way, this is nothing new.
thanks,
greg k-h
^ permalink raw reply
* Re: [BUG] skge 0000:02:05: read data parity error
From: Adrian Bunk @ 2008-01-14 21:52 UTC (permalink / raw)
To: Oliver Pinter (Pintér Olivér)
Cc: Stephen Hemminger, linux-kernel, netdev, gregkh
In-Reply-To: <6101e8c40801141341hcb17b08h2fd5244bccb21928@mail.gmail.com>
On Mon, Jan 14, 2008 at 10:41:52PM +0100, Oliver Pinter (Pintér Olivér) wrote:
> On 1/14/08, Adrian Bunk <bunk@kernel.org> wrote:
> > On Mon, Jan 14, 2008 at 10:31:03PM +0100, Oliver Pinter (Pintér Olivér)
> > wrote:
> > > I think, it is a potential security breakpoint, when applications with
> > > root permission its read, then a machine is freezed, or only i thin
> > > it's?
> >
> > When you are root there are infinite ways to kill your machine, so
> > there's nothing security related about this issue.
>
> Yes, i know, but when some application or daemons read some file with
> running root privileges, then ...
>
> thanks, then it is only a "feature" and not bug.
It might be a bug in the application.
But there are worse things than crashing your machine (e.g. getting your
/etc/shadow) that can happen when someone with bad intentions can read
files with root privileges.
> Thanks,
> Oliver
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
^ permalink raw reply
* Re: [BUG] skge 0000:02:05: read data parity error
From: Oliver Pinter (Pintér Olivér) @ 2008-01-14 21:58 UTC (permalink / raw)
To: Adrian Bunk; +Cc: Stephen Hemminger, linux-kernel, netdev, gregkh
In-Reply-To: <20080114215209.GK9847@does.not.exist>
I "tested" other devices resources file, and only with skge freezed
the system. from this think, that is skge driver bug
--
Thanks,
Oliver
^ permalink raw reply
* Re: [PATCH 0/3] bonding: 3 fixes for 2.6.24
From: Krzysztof Oledzki @ 2008-01-14 22:15 UTC (permalink / raw)
To: Jay Vosburgh
Cc: Andy Gospodarek, netdev, Jeff Garzik, David Miller, Herbert Xu
In-Reply-To: <18609.1200160598@death>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 2394 bytes --]
On Sat, 12 Jan 2008, Jay Vosburgh wrote:
> Krzysztof Oledzki <olel@ans.pl> wrote:
> [...]
>> Exactly. All I need to do is to reboot my server, I have 100% probability
>> to get the warning.
>
> I wish it were that easy for me; I'm not sure what magic thing
> you've got on your server or network that I don't, but I haven't been
> able to make this lockdep warning happen at all.
>
>> Right. So, what is the final patch? I would like to test it if that's
>> possible. ;)
>
> Can you test the following and let me know if it triggers the
> warning? I believe this is the minimum locking needed, and based on
> input from Herbert, we shouldn't need to hold the lock at _bh. If this
> one works, and nobody sees any other issues with it, then it's the final
> patch for this lockdep problem. I'll add some deep, meaningful comments
> to explain the locking a bit (i.e., we're called with rtnl for the
> allmulti and promisc cases, so we're ok there without additional locks,
> but the later code could be called from anywhere, so it needs locks to
> prevent the slave list from changing, but the mc_lists themselves are
> covered by the netif_tx_lock that all callers will hold), but this would
> be the actual code change.
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 77d004d..6906dbc 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -3937,8 +3937,6 @@ static void bond_set_multicast_list(struct net_device *bond_dev)
> struct bonding *bond = bond_dev->priv;
> struct dev_mc_list *dmi;
>
> - write_lock_bh(&bond->lock);
> -
> /*
> * Do promisc before checking multicast_mode
> */
> @@ -3959,6 +3957,8 @@ static void bond_set_multicast_list(struct net_device *bond_dev)
> bond_set_allmulti(bond, -1);
> }
>
> + read_lock(&bond->lock);
> +
> bond->flags = bond_dev->flags;
>
> /* looking for addresses to add to slaves' mc list */
> @@ -3979,7 +3979,7 @@ static void bond_set_multicast_list(struct net_device *bond_dev)
> bond_mc_list_destroy(bond);
> bond_mc_list_copy(bond_dev->mc_list, bond, GFP_ATOMIC);
>
> - write_unlock_bh(&bond->lock);
> + read_unlock(&bond->lock);
> }
>
> /*
>
>
I can confirm that the warning went away.
Tested-by: Krzysztof Piotr Oledzki <ole@ans.pl>
Best regards,
Krzysztof Olędzki
^ permalink raw reply
* [PATCH 2/5] forcedeth: checksum fix
From: Ayaz Abdulla @ 2008-01-13 21:02 UTC (permalink / raw)
To: Jeff Garzik, Manfred Spraul, Andrew Morton, nedev
[-- Attachment #1: Type: text/plain, Size: 625 bytes --]
The driver should inform the stack when checksum has been performed by
the HW when both IP and TCP (or UDP) checksum flags are indicated by HW.
Previously, it would also inform the stack when only IP checksum flag
was indicated by HW. This can cause data corruption when IP fragments
are used. The IP Identification field can wrap around and cause data
from new fragments to fill into older fragment slots with same IP Id.
The stack would then not perform TCP/UDP checksum (after re-assembly of
all fragments) since driver falsely stated it was already calculated.
Signed-off-by: Ayaz Abdulla <aabdulla@nvidia.com>
[-- Attachment #2: patch-forcedeth-checksum --]
[-- Type: text/plain, Size: 1826 bytes --]
--- old/drivers/net/forcedeth.c 2008-01-13 15:01:36.000000000 -0500
+++ new/drivers/net/forcedeth.c 2008-01-13 15:08:31.000000000 -0500
@@ -471,9 +471,9 @@
#define NV_RX_AVAIL (1<<31)
#define NV_RX2_CHECKSUMMASK (0x1C000000)
-#define NV_RX2_CHECKSUMOK1 (0x10000000)
-#define NV_RX2_CHECKSUMOK2 (0x14000000)
-#define NV_RX2_CHECKSUMOK3 (0x18000000)
+#define NV_RX2_CHECKSUM_IP (0x10000000)
+#define NV_RX2_CHECKSUM_IP_TCP (0x14000000)
+#define NV_RX2_CHECKSUM_IP_UDP (0x18000000)
#define NV_RX2_DESCRIPTORVALID (1<<29)
#define NV_RX2_SUBSTRACT1 (1<<25)
#define NV_RX2_ERROR1 (1<<18)
@@ -2375,14 +2375,9 @@
goto next_pkt;
}
}
- if ((flags & NV_RX2_CHECKSUMMASK) == NV_RX2_CHECKSUMOK2)/*ip and tcp */ {
+ if (((flags & NV_RX2_CHECKSUMMASK) == NV_RX2_CHECKSUM_IP_TCP) || /*ip and tcp */
+ ((flags & NV_RX2_CHECKSUMMASK) == NV_RX2_CHECKSUM_IP_UDP)) /*ip and udp */
skb->ip_summed = CHECKSUM_UNNECESSARY;
- } else {
- if ((flags & NV_RX2_CHECKSUMMASK) == NV_RX2_CHECKSUMOK1 ||
- (flags & NV_RX2_CHECKSUMMASK) == NV_RX2_CHECKSUMOK3) {
- skb->ip_summed = CHECKSUM_UNNECESSARY;
- }
- }
} else {
dev_kfree_skb(skb);
goto next_pkt;
@@ -2474,14 +2469,9 @@
}
}
- if ((flags & NV_RX2_CHECKSUMMASK) == NV_RX2_CHECKSUMOK2)/*ip and tcp */ {
+ if (((flags & NV_RX2_CHECKSUMMASK) == NV_RX2_CHECKSUM_IP_TCP) || /*ip and tcp */
+ ((flags & NV_RX2_CHECKSUMMASK) == NV_RX2_CHECKSUM_IP_UDP)) /*ip and udp */
skb->ip_summed = CHECKSUM_UNNECESSARY;
- } else {
- if ((flags & NV_RX2_CHECKSUMMASK) == NV_RX2_CHECKSUMOK1 ||
- (flags & NV_RX2_CHECKSUMMASK) == NV_RX2_CHECKSUMOK3) {
- skb->ip_summed = CHECKSUM_UNNECESSARY;
- }
- }
/* got a valid packet - forward it to the network core */
skb_put(skb, len);
^ 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