Netdev List
 help / color / mirror / Atom feed
* 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: 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: [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: 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: [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: [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: 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 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: [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

* [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: [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

* 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: 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: [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: 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

* 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: 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

* Re: questions on NAPI processing latency and dropped network packets
From: Chris Friesen @ 2008-01-14 20:02 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Ray Lee, netdev, linux-kernel
In-Reply-To: <478BB904.3060903@cosmosbay.com>

Eric Dumazet wrote:
> Chris Friesen a écrit :

>> Based on the profiling information we're spending time in 
>> sctp_endpoint_lookup_assoc() which doesn't actually use hashes, so I 
>> can't see how the hash would be related.  I'm pretty new to SCTP 
>> though, so I may be missing something.
> 
> Well, it does use hashes :)
> 
>        hash = sctp_assoc_hashfn(ep->base.bind_addr.port, rport);
>        head = &sctp_assoc_hashtable[hash];
>        read_lock(&head->lock);
>        sctp_for_each_hentry(epb, node, &head->chain) {
>            /* maybe your machine is traversing here a *really* long 
> chain */
>            }


The latest released kernel doesn't have this code, it was only added in 
November.  The SCTP maintainer just pointed me to the patch, and made 
some other suggestions as well.

Chris

^ permalink raw reply

* Re: 2.6.24-rc6-mm1 - oddness with IPv4/v6 mapped sockets hanging...
From: Paul Moore @ 2008-01-14 20:02 UTC (permalink / raw)
  To: Valdis.Kletnieks; +Cc: Andrew Morton, linux-kernel, netdev
In-Reply-To: <3727.1200339422@turing-police.cc.vt.edu>

On Monday 14 January 2008 2:37:02 pm Valdis.Kletnieks@vt.edu wrote:
> On Mon, 14 Jan 2008 14:07:46 EST, Paul Moore said:
> > There have been quite a few changes in lblnet-2.6_testing since
> > 2.6.24-rc6-mm1 so I would recommend taking the whole tree.  I'm also not
> > quite sure if
>
> Weird.  I did a 'git clone
> git://git.infradead.org/users/pcmoore/lblnet-2.6_testing' into a new
> directory this morning, and doing a 'git log' against that only showed the
> one added commit:
>
> commit 5d95575903fd3865b884952bd93c339d48725c33
> Author: Paul Moore <paul.moore@hp.com>
> Date:   Wed Jan 9 15:30:23 2008 -0500
>
>     SELinux: Add warning messages on network denial due to error
>
>     Currently network traffic can be sliently dropped due to non-avc errors
> which can lead to much confusion when trying to debug the problem.  This
> patch adds warning messages so that when these events occur there is a user
> visible notification.
>
>     Signed-off-by: Paul Moore <paul.moore@hp.com>
>
> commit 9259ca5fd8b9fbdd2c3edade593dead905d8391e
> Author: Paul Moore <paul.moore@hp.com>
> Date:   Wed Jan 9 15:30:23 2008 -0500
>
>     SELinux: Add network ingress and egress control permission checks
> (already in 24-rc6-mm1).
>
> Somebody please tell me it's my git-idiocy..

It might be something on my end with managing the lblnet-2.6_testing git tree; 
I'm still pretty clueless when it comes to git.

I've got a git tree on my dev machine which is backed against Linus' tree and 
managed via stacked-git.  I update the patches in this tree, refresh them 
against new bits from Linus, etc and when something significant changes I 
update the git tree on infradead.org and post a new patchset to the related 
lists.  The process of updating the git tree on infradead.org usually 
involves deleting the entire tree located there, re-creating it, and then 
doing a git-push from my dev machine.  I have no idea if this is "correct" or 
not, but I've often wondered if this is a the "right" way to do it ...

-- 
paul moore
linux security @ hp

^ permalink raw reply

* [BUG] skge 0000:02:05: read data parity error
From: Oliver Pinter (Pintér Olivér) @ 2008-01-14 19:57 UTC (permalink / raw)
  To: linux-kernel; +Cc: shemminger, netdev, oliver.pntr

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

when I used cat for show what is in file , then bocome this message:
 cat: resource0: Input/output error
but the permissions is:
 -rw------- 1 root root 16384 2008-01-14 20:36 resource0

when I used mcview for show whats in file, then the system hang up,
and not reagiert neither for SysRQ-s, only for hard reset.

-----------

PC1.:
00:00.0 Host bridge [0600]: Intel Corporation 945G/GZ/P/PL Express
Memory Controller Hub [8086:2770] (rev 02)
00:01.0 PCI bridge [0604]: Intel Corporation 945G/GZ/P/PL Express PCI
Express Root Port [8086:2771] (rev 02)
00:1b.0 Audio device [0403]: Intel Corporation 82801G (ICH7 Family)
High Definition Audio Controller [8086:27d8] (rev 01)
00:1d.0 USB Controller [0c03]: Intel Corporation 82801G (ICH7 Family)
USB UHCI #1 [8086:27c8] (rev 01)
00:1d.1 USB Controller [0c03]: Intel Corporation 82801G (ICH7 Family)
USB UHCI #2 [8086:27c9] (rev 01)
00:1d.2 USB Controller [0c03]: Intel Corporation 82801G (ICH7 Family)
USB UHCI #3 [8086:27ca] (rev 01)
00:1d.3 USB Controller [0c03]: Intel Corporation 82801G (ICH7 Family)
USB UHCI #4 [8086:27cb] (rev 01)
00:1d.7 USB Controller [0c03]: Intel Corporation 82801G (ICH7 Family)
USB2 EHCI Controller [8086:27cc] (rev 01)
00:1e.0 PCI bridge [0604]: Intel Corporation 82801 PCI Bridge
[8086:244e] (rev e1)
00:1f.0 ISA bridge [0601]: Intel Corporation 82801GB/GR (ICH7 Family)
LPC Interface Bridge [8086:27b8] (rev 01)
00:1f.2 IDE interface [0101]: Intel Corporation 82801GB/GR/GH (ICH7
Family) Serial ATA Storage Controller IDE [8086:27c0] (rev 01)
00:1f.3 SMBus [0c05]: Intel Corporation 82801G (ICH7 Family) SMBus
Controller [8086:27da] (rev 01)
01:00.0 VGA compatible controller [0300]: nVidia Corporation GeForce
7300 GS [10de:01df] (rev a1)
02:01.0 Ethernet controller [0200]: Atheros Communications, Inc.
AR5212 802.11abg NIC [168c:0013] (rev 01)
02:05.0 Ethernet controller [0200]: Marvell Technology Group Ltd.
88E8001 Gigabit Ethernet Controller [11ab:4320] (rev 13)

PC2:
00:00.0 Host bridge [0600]: Intel Corporation 82875P/E7210 Memory
Controller Hub [8086:2578] (rev 02)
00:01.0 PCI bridge [0604]: Intel Corporation 82875P Processor to AGP
Controller [8086:2579] (rev 02)
00:06.0 System peripheral [0880]: Intel Corporation 82875P/E7210
Processor to I/O Memory Interface [8086:257e] (rev 02)
00:1d.0 USB Controller [0c03]: Intel Corporation 82801EB/ER
(ICH5/ICH5R) USB UHCI Controller #1 [8086:24d2] (rev 02)
00:1d.1 USB Controller [0c03]: Intel Corporation 82801EB/ER
(ICH5/ICH5R) USB UHCI Controller #2 [8086:24d4] (rev 02)
00:1d.7 USB Controller [0c03]: Intel Corporation 82801EB/ER
(ICH5/ICH5R) USB2 EHCI Controller [8086:24dd] (rev 02)
00:1e.0 PCI bridge [0604]: Intel Corporation 82801 PCI Bridge
[8086:244e] (rev c2)
00:1f.0 ISA bridge [0601]: Intel Corporation 82801EB/ER (ICH5/ICH5R)
LPC Interface Bridge [8086:24d0] (rev 02)
00:1f.1 IDE interface [0101]: Intel Corporation 82801EB/ER
(ICH5/ICH5R) IDE Controller [8086:24db] (rev 02)
00:1f.2 RAID bus controller [0104]: Intel Corporation 82801ER (ICH5R)
SATA Controller [8086:24df] (rev 02)
00:1f.3 SMBus [0c05]: Intel Corporation 82801EB/ER (ICH5/ICH5R) SMBus
Controller [8086:24d3] (rev 02)
00:1f.5 Multimedia audio controller [0401]: Intel Corporation
82801EB/ER (ICH5/ICH5R) AC'97 Audio Controller [8086:24d5] (rev 02)
01:00.0 VGA compatible controller [0300]: ATI Technologies Inc RV350
AP [Radeon 9600] [1002:4150]
01:00.1 Display controller [0380]: ATI Technologies Inc RV350 AP
[Radeon 9600] (Secondary) [1002:4170]
02:05.0 Ethernet controller [0200]: 3Com Corporation 3c940
10/100/1000Base-T [Marvell] [10b7:1700] (rev 12)
02:0a.0 Ethernet controller [0200]: Atheros Communications, Inc.
AR5212/AR5213 Multiprotocol MAC/baseband processor [168c:0013] (rev
01)
-- 
Thanks,
Oliver

^ permalink raw reply

* Re: occasionally corrupted network stats in /proc/net/dev
From: Michael Chan @ 2008-01-14 20:41 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Mark Seger, Ben Greear, netdev
In-Reply-To: <478BB43B.8060905@cosmosbay.com>

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

* Re: [PATCH 2/2] ixgb: enable sun hardware support for broadcom phy
From: Matheos Worku @ 2008-01-14 19:43 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: Auke Kok, netdev, jesse.brandeburg
In-Reply-To: <47671FBD.4010402@garzik.org>

Jeff Garzik wrote:
> Auke Kok wrote:
>> From: Matheos Worku <matheos.worku@sun.com>
>>
>> Implement support for a SUN-specific PHY.
>>
>> SUN provides a modified 82597-based board with their own
>> PHY that works with very little modification to the code. This
>> patch implements this new PHY which is identified by the
>> subvendor device ID. The device ID of the adapter remains
>> the same.
>>
>> Signed-off-by: Matheos Worku <matheos.worku@sun.com>
>> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
>> Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
>> ---
>>
>>  drivers/net/ixgb/ixgb_hw.c   |   82 
>> +++++++++++++++++++++++++++++++++++++++++-
>>  drivers/net/ixgb/ixgb_hw.h   |    3 +-
>>  drivers/net/ixgb/ixgb_ids.h  |    4 ++
>>  drivers/net/ixgb/ixgb_main.c |   10 +++--
>>  4 files changed, 91 insertions(+), 8 deletions(-)
> 
> applied #upstream
> 
> 

Jeff,
Bleated Happy New Year.

I was wondering when this patch will  make  it to Linus' and other 
trees. I just checked Linus's and other net trees, including 
git.kernel.org/?p=linux/kernel/git/jgarzik/netdev-2.6.git and I see only 
the other patch's commit: 3fd7131feacc01c1e23e46c416228f36ebdcc0d4, and 
not this patch.


Regards
Matheos

^ permalink raw reply

* Re: 2.6.24-rc6-mm1 - oddness with IPv4/v6 mapped sockets hanging...
From: Valdis.Kletnieks @ 2008-01-14 19:37 UTC (permalink / raw)
  To: Paul Moore; +Cc: Andrew Morton, linux-kernel, netdev
In-Reply-To: <200801141407.46345.paul.moore@hp.com>

[-- Attachment #1: Type: text/plain, Size: 1779 bytes --]

On Mon, 14 Jan 2008 14:07:46 EST, Paul Moore said:
> There have been quite a few changes in lblnet-2.6_testing since 2.6.24-rc6-mm1 
> so I would recommend taking the whole tree.  I'm also not quite sure if

Weird.  I did a 'git clone git://git.infradead.org/users/pcmoore/lblnet-2.6_testing'
into a new directory this morning, and doing a 'git log' against that only
showed the one added commit:

commit 5d95575903fd3865b884952bd93c339d48725c33
Author: Paul Moore <paul.moore@hp.com>
Date:   Wed Jan 9 15:30:23 2008 -0500

    SELinux: Add warning messages on network denial due to error
    
    Currently network traffic can be sliently dropped due to non-avc errors which
    can lead to much confusion when trying to debug the problem.  This patch adds
    warning messages so that when these events occur there is a user visible
    notification.
    
    Signed-off-by: Paul Moore <paul.moore@hp.com>

commit 9259ca5fd8b9fbdd2c3edade593dead905d8391e
Author: Paul Moore <paul.moore@hp.com>
Date:   Wed Jan 9 15:30:23 2008 -0500

    SELinux: Add network ingress and egress control permission checks
(already in 24-rc6-mm1).

Somebody please tell me it's my git-idiocy..

> simply reverting the "Convert the netif code to use ifindex values" patch 
> would solve the problem as there are other patches in the rc6-mm1 tree that 
> rely on skb->iif being valid (new code, not converted code).

That would explain why I'm still seeing issues..

>  If you want to 
> stick with a _relatively_ vanilla rc6-mm1 tree I would leave everything in 
> and simply apply the following patch which solved the skb_clone()/iif 
> problem:
> 
> http://git.infradead.org/?p=users/pcmoore/lblnet-2.6_testing;a=commitdiff;h=02f1c89d6e36507476f78108a3dcc78538be460b

OK, I'll go look at that..


[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

^ permalink raw reply

* Re: questions on NAPI processing latency and dropped network packets
From: Eric Dumazet @ 2008-01-14 19:33 UTC (permalink / raw)
  To: Chris Friesen; +Cc: Ray Lee, netdev, linux-kernel
In-Reply-To: <478BB722.1020004@nortel.com>

Chris Friesen a écrit :
> Eric Dumazet wrote:
>> Chris Friesen a écrit :
>
>>> Based on profiling and instrumentation it seems like the cost of 
>>> sctp_endpoint_lookup_assoc() more than triples, which means that the 
>>> amount of time that bottom halves are disabled in that function also 
>>> triples.
>>
>> Any idea of the size of sctp hash size you have ?
>> (your dmesg probably includes a message starting with SCTP: Hash 
>> tables configured...
>> How many concurrent sctp sockets are handled ?
>
> Our lab is currently rebooting, but I'll try and get this once it's 
> back up.
>
>> Maybe sctp_assoc_hashfn() is too weak for your use, and some chains 
>> are *really* long.
>
> Based on the profiling information we're spending time in 
> sctp_endpoint_lookup_assoc() which doesn't actually use hashes, so I 
> can't see how the hash would be related.  I'm pretty new to SCTP 
> though, so I may be missing something.
Well, it does use hashes :)

        hash = sctp_assoc_hashfn(ep->base.bind_addr.port, rport);
        head = &sctp_assoc_hashtable[hash];
        read_lock(&head->lock);
        sctp_for_each_hentry(epb, node, &head->chain) {
            /* maybe your machine is traversing here a *really* long 
chain */
            }

>
> Here's the top results from readprofile, unfortunately these are 
> aggregated across both cpus so they don't really show what's going on. 
> The key thing is that sctp_endpoint_lookup_assoc() is the most 
> expensive kernel routine on this entire system.
>
>   3147 .power4_idle                              22.4786
>   1712 .native_idle                              20.3810
>   1234 .sctp_endpoint_lookup_assoc                2.1725

>   1212 ._spin_unlock_irqrestore                   6.4468
>    778 .do_futex                                  0.3791
>    447 ._spin_unlock_irq                          4.2981
>    313 .fget                                      1.7784
>    277 .fput                                      3.8472
>    275 .kfree                                     0.7473
>    234 .__kmalloc                                 0.5571
>    131 SystemCall_common                          0.3411
>    130 .sctp_assoc_is_match                       0.6373
>    123 .lock_sock                                 0.4155
>    119 .find_vma                                  0.6919
>    116 .kmem_cache_alloc                          0.3580
>    111 .kmem_cache_free                           0.3343
>    106 .skb_release_data                          0.4907
>    102 .__copy_tofrom_user                        0.0724
>    100 .exit_elf_binfmt                           1.9231
>    100 .do_select                                 0.0820
>
>
> Chris
> -- 
> 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

* [FIB]: Avoid using static variables without proper locking
From: Eric Dumazet @ 2008-01-14 19:27 UTC (permalink / raw)
  To: David Miller; +Cc: Robert Olsson, netdev
In-Reply-To: <18315.41725.417992.715140@robur.slu.se>

[-- Attachment #1: Type: text/plain, Size: 287 bytes --]

fib_trie_seq_show() uses two helper functions, rtn_scope() and 
rtn_type() that can
write to static storage without locking.

Just pass to them a temporary buffer to avoid potential  corruption
(probably not triggerable but still...)

Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>



[-- Attachment #2: fib_trie.patch --]
[-- Type: text/plain, Size: 1936 bytes --]

diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index 8d8c291..15a555a 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -2276,10 +2276,8 @@ static void seq_indent(struct seq_file *seq, int n)
 	while (n-- > 0) seq_puts(seq, "   ");
 }
 
-static inline const char *rtn_scope(enum rt_scope_t s)
+static inline const char *rtn_scope(char *buf, size_t len, enum rt_scope_t s)
 {
-	static char buf[32];
-
 	switch (s) {
 	case RT_SCOPE_UNIVERSE: return "universe";
 	case RT_SCOPE_SITE:	return "site";
@@ -2287,7 +2285,7 @@ static inline const char *rtn_scope(enum rt_scope_t s)
 	case RT_SCOPE_HOST:	return "host";
 	case RT_SCOPE_NOWHERE:	return "nowhere";
 	default:
-		snprintf(buf, sizeof(buf), "scope=%d", s);
+		snprintf(buf, len, "scope=%d", s);
 		return buf;
 	}
 }
@@ -2307,13 +2305,11 @@ static const char *rtn_type_names[__RTN_MAX] = {
 	[RTN_XRESOLVE] = "XRESOLVE",
 };
 
-static inline const char *rtn_type(unsigned t)
+static inline const char *rtn_type(char *buf, size_t len, unsigned t)
 {
-	static char buf[32];
-
 	if (t < __RTN_MAX && rtn_type_names[t])
 		return rtn_type_names[t];
-	snprintf(buf, sizeof(buf), "type %d", t);
+	snprintf(buf, len, "type %d", t);
 	return buf;
 }
 
@@ -2351,13 +2347,19 @@ static int fib_trie_seq_show(struct seq_file *seq, void *v)
 		seq_printf(seq, "  |-- %d.%d.%d.%d\n", NIPQUAD(val));
 		for (i = 32; i >= 0; i--) {
 			struct leaf_info *li = find_leaf_info(l, i);
+
 			if (li) {
 				struct fib_alias *fa;
+
 				list_for_each_entry_rcu(fa, &li->falh, fa_list) {
+					char buf1[32], buf2[32];
+
 					seq_indent(seq, iter->depth+1);
 					seq_printf(seq, "  /%d %s %s", i,
-						   rtn_scope(fa->fa_scope),
-						   rtn_type(fa->fa_type));
+						   rtn_scope(buf1, sizeof(buf1),
+							     fa->fa_scope),
+						   rtn_type(buf2, sizeof(buf2),
+							     fa->fa_type));
 					if (fa->fa_tos)
 						seq_printf(seq, "tos =%d\n",
 							   fa->fa_tos);

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox