* Re: 2.6.16-rc5-mm2: IPW_QOS: two remarks
From: John W. Linville @ 2006-03-17 19:14 UTC (permalink / raw)
To: Andreas Happe, Adrian Bunk
Cc: Andrew Morton, linux-kernel, jgarzik, netdev, zhu.yi
In-Reply-To: <200603050146.27529.andreashappe@snikt.net>
On Sun, Mar 05, 2006 at 01:46:26AM +0100, Andreas Happe wrote:
> Add the following config entries for the ipw2200 driver to
> drivers/net/wireless/Kconfig
> * IPW2200_MONITOR
> enables Monitor mode, as this seems to generate lots of firmware errors
> it depends upon BROKEN
> * IPW2200_QOS
> enables QoS feature - this is under development right now, so it depends
> upon EXPERIMENTAL.
Your patch appears to be whitespace-damaged. Please configure your
mailer appropriately.
Also, please stick to the patch format described here:
http://linux.yyz.us/patch-format.html
In particular, don't put anything in the message that doesn't belong
in the kernel's changelog, such as email-ish messages.
On Tue, Mar 07, 2006 at 06:06:42PM +0100, Adrian Bunk wrote:
> On Sun, Mar 05, 2006 at 01:46:26AM +0100, Andreas Happe wrote:
> > On Friday 03 March 2006 16:26, Adrian Bunk wrote:
> >...
> > > - please add a help text
> >
> > i could add some stuff about WMM to its help text, but I think someone more
> > involved with the ipw2200-project should do that.
>
> Even a short help text is better than no help text.
I agree w/ Adrian. Since you are touching it, please put something
appropriate in the Kconfig file. Zhu Yi may be able to help if you
aren't sure what the help text should say.
Thanks!
John
--
John W. Linville
linville@tuxdriver.com
^ permalink raw reply
* Re: drivers/net/chelsio/sge.c: two array overflows
From: Scott Bardone @ 2006-03-17 18:46 UTC (permalink / raw)
To: Hans-Peter Jansen
Cc: Jeff Garzik, Adrian Bunk, maintainers, netdev, linux-kernel
In-Reply-To: <200603171319.20935.hpj@urpla.net>
Thanks Pete,
This is correct, the array should contain 3 elements. The bug was we were
accessing a 4th element ([3]) which did not exist. We should be modifying the
last element ([2]) instead.
-Scott
Hans-Peter Jansen wrote:
> [from the nitpick department..]
>
> Hi Jeff, hi Scott,
>
> Adrian wrote:
>
>>The Coverity checker spotted the following two array overflows in
>>drivers/net/chelsio/sge.c (in both cases, the arrays contain 3
>>elements):
>
>
> Am Freitag, 17. März 2006 01:21 schrieb Jeff Garzik:
>
>>Scott Bardone wrote:
>>
>>>Adrian,
>>>
>>>This is a bug. The array should contain 2 elements.
>>>
>>>Attached is a patch which fixes it.
>>>Thanks.
>>>
>>>Signed-off-by: Scott Bardone <sbardone@chelsio.com>
>>
>>applied. please avoid attachments and use a proper patch description
>>in the future. I had to hand-edit and hand-apply your patch.
>
>
> where you wrote in kernel tree commit
> 347a444e687b5f8cf0f6485704db1c6024d3:
> This is a bug. The array should contain 2 elements. Here is the fix.
>
> If I'm not completely off the track, you both committed a description
> off by one error: since the patch doesn't change the array size, it's
> presumely¹ still 3 elements, where index 2 references the last one.
>
> Here's hopefully a better patch description:
> Fixed off by one thinko in stats accounting, spotted by Coverity
> checker, notified by Adrian "The Cleanman" Bunk.
>
> SCR,
> Pete
>
> ¹) otherwise, it's still off by one..
^ permalink raw reply
* Re: drivers/net/chelsio/sge.c: two array overflows
From: Hans-Peter Jansen @ 2006-03-17 12:19 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Scott Bardone, Adrian Bunk, maintainers, netdev, linux-kernel
In-Reply-To: <441A011A.6010705@pobox.com>
[from the nitpick department..]
Hi Jeff, hi Scott,
Adrian wrote:
>The Coverity checker spotted the following two array overflows in
>drivers/net/chelsio/sge.c (in both cases, the arrays contain 3
>elements):
Am Freitag, 17. März 2006 01:21 schrieb Jeff Garzik:
> Scott Bardone wrote:
> > Adrian,
> >
> > This is a bug. The array should contain 2 elements.
> >
> > Attached is a patch which fixes it.
> > Thanks.
> >
> > Signed-off-by: Scott Bardone <sbardone@chelsio.com>
>
> applied. please avoid attachments and use a proper patch description
> in the future. I had to hand-edit and hand-apply your patch.
where you wrote in kernel tree commit
347a444e687b5f8cf0f6485704db1c6024d3:
This is a bug. The array should contain 2 elements. Here is the fix.
If I'm not completely off the track, you both committed a description
off by one error: since the patch doesn't change the array size, it's
presumely¹ still 3 elements, where index 2 references the last one.
Here's hopefully a better patch description:
Fixed off by one thinko in stats accounting, spotted by Coverity
checker, notified by Adrian "The Cleanman" Bunk.
SCR,
Pete
¹) otherwise, it's still off by one..
^ permalink raw reply
* Re: [PATCH 1/8] [I/OAT] DMA memcpy subsystem
From: Kumar Gala @ 2006-03-17 7:30 UTC (permalink / raw)
To: Chris Leech; +Cc: linux kernel mailing list, netdev
In-Reply-To: <20060311022919.3950.43835.stgit@gitlost.site>
[snip]
> +/**
> + * dma_async_client_register - allocate and register a &dma_client
> + * @event_callback: callback for notification of channel addition/
> removal
> + */
> +struct dma_client *dma_async_client_register(dma_event_callback
> event_callback)
> +{
> + struct dma_client *client;
> +
> + client = kzalloc(sizeof(*client), GFP_KERNEL);
> + if (!client)
> + return NULL;
> +
> + INIT_LIST_HEAD(&client->channels);
> + spin_lock_init(&client->lock);
> +
> + client->chans_desired = 0;
> + client->chan_count = 0;
> + client->event_callback = event_callback;
> +
> + spin_lock(&dma_list_lock);
> + list_add_tail(&client->global_node, &dma_client_list);
> + spin_unlock(&dma_list_lock);
> +
> + return client;
> +}
It would seem that when a client registers (or shortly there after
when they call dma_async_client_chan_request()) they would expect to
get the number of channels they need by some given time period.
For example, lets say a client registers but no dma device exists.
They will never get called to be aware of this condition.
I would think most clients would either spin until they have all the
channels they need or fall back to a non-async mechanism.
Also, what do you think about adding an operation type (MEMCPY, XOR,
CRYPTO_AES, etc). We can than validate if the operation type
expected is supported by the devices that exist.
> +
> +/**
> + * dma_async_client_unregister - unregister a client and free the
> &dma_client
> + * @client:
> + *
> + * Force frees any allocated DMA channels, frees the &dma_client
> memory
> + */
> +void dma_async_client_unregister(struct dma_client *client)
> +{
> + struct dma_chan *chan;
> +
> + if (!client)
> + return;
> +
> + rcu_read_lock();
> + list_for_each_entry_rcu(chan, &client->channels, client_node) {
> + dma_client_chan_free(chan);
> + }
> + rcu_read_unlock();
> +
> + spin_lock(&dma_list_lock);
> + list_del(&client->global_node);
> + spin_unlock(&dma_list_lock);
> +
> + kfree(client);
> + dma_chans_rebalance();
> +}
> +
> +/**
> + * dma_async_client_chan_request - request DMA channels
> + * @client: &dma_client
> + * @number: count of DMA channels requested
> + *
> + * Clients call dma_async_client_chan_request() to specify how many
> + * DMA channels they need, 0 to free all currently allocated.
> + * The resulting allocations/frees are indicated to the client via
> the
> + * event callback.
> + */
> +void dma_async_client_chan_request(struct dma_client *client,
> + unsigned int number)
> +{
> + client->chans_desired = number;
> + dma_chans_rebalance();
> +}
> +
Shouldn't we also have a dma_async_client_chan_free()?
[snip]
> +/* --- public DMA engine API --- */
> +
> +struct dma_client *dma_async_client_register(dma_event_callback
> event_callback);
> +void dma_async_client_unregister(struct dma_client *client);
> +void dma_async_client_chan_request(struct dma_client *client,
> + unsigned int number);
> +
> +/**
> + * dma_async_memcpy_buf_to_buf - offloaded copy between virtual
> addresses
> + * @chan: DMA channel to offload copy to
> + * @dest: destination address (virtual)
> + * @src: source address (virtual)
> + * @len: length
> + *
> + * Both @dest and @src must be mappable to a bus address according
> to the
> + * DMA mapping API rules for streaming mappings.
> + * Both @dest and @src must stay memory resident (kernel memory or
> locked
> + * user space pages)
> + */
> +static inline dma_cookie_t dma_async_memcpy_buf_to_buf(struct
> dma_chan *chan,
> + void *dest, void *src, size_t len)
> +{
> + int cpu = get_cpu();
> + per_cpu_ptr(chan->local, cpu)->bytes_transferred += len;
> + per_cpu_ptr(chan->local, cpu)->memcpy_count++;
> + put_cpu();
> +
> + return chan->device->device_memcpy_buf_to_buf(chan, dest, src, len);
> +}
What about renaming the dma_async_memcpy_* to something like
dma_async_op_* and have them take an additional operation argument.
> +
> +/**
> + * dma_async_memcpy_buf_to_pg - offloaded copy
> + * @chan: DMA channel to offload copy to
> + * @page: destination page
> + * @offset: offset in page to copy to
> + * @kdata: source address (virtual)
> + * @len: length
> + *
> + * Both @page/@offset and @kdata must be mappable to a bus address
> according
> + * to the DMA mapping API rules for streaming mappings.
> + * Both @page/@offset and @kdata must stay memory resident (kernel
> memory or
> + * locked user space pages)
> + */
> +static inline dma_cookie_t dma_async_memcpy_buf_to_pg(struct
> dma_chan *chan,
> + struct page *page, unsigned int offset, void *kdata, size_t len)
> +{
> + int cpu = get_cpu();
> + per_cpu_ptr(chan->local, cpu)->bytes_transferred += len;
> + per_cpu_ptr(chan->local, cpu)->memcpy_count++;
> + put_cpu();
> +
> + return chan->device->device_memcpy_buf_to_pg(chan, page, offset,
> + kdata, len);
> +}
> +
> +/**
> + * dma_async_memcpy_buf_to_pg - offloaded copy
> + * @chan: DMA channel to offload copy to
> + * @dest_page: destination page
> + * @dest_off: offset in page to copy to
> + * @src_page: source page
> + * @src_off: offset in page to copy from
> + * @len: length
> + *
> + * Both @dest_page/@dest_off and @src_page/@src_off must be
> mappable to a bus
> + * address according to the DMA mapping API rules for streaming
> mappings.
> + * Both @dest_page/@dest_off and @src_page/@src_off must stay
> memory resident
> + * (kernel memory or locked user space pages)
> + */
> +static inline dma_cookie_t dma_async_memcpy_pg_to_pg(struct
> dma_chan *chan,
> + struct page *dest_pg, unsigned int dest_off, struct page *src_pg,
> + unsigned int src_off, size_t len)
> +{
> + int cpu = get_cpu();
> + per_cpu_ptr(chan->local, cpu)->bytes_transferred += len;
> + per_cpu_ptr(chan->local, cpu)->memcpy_count++;
> + put_cpu();
> +
> + return chan->device->device_memcpy_pg_to_pg(chan, dest_pg, dest_off,
> + src_pg, src_off, len);
> +}
> +
> +/**
> + * dma_async_memcpy_issue_pending - flush pending copies to HW
> + * @chan:
> + *
> + * This allows drivers to push copies to HW in batches,
> + * reducing MMIO writes where possible.
> + */
> +static inline void dma_async_memcpy_issue_pending(struct dma_chan
> *chan)
> +{
> + return chan->device->device_memcpy_issue_pending(chan);
> +}
> +
> +/**
> + * dma_async_memcpy_complete - poll for transaction completion
> + * @chan: DMA channel
> + * @cookie: transaction identifier to check status of
> + * @last: returns last completed cookie, can be NULL
> + * @used: returns last issued cookie, can be NULL
> + *
> + * If @last and @used are passed in, upon return they reflect the
> driver
> + * internal state and can be used with dma_async_is_complete() to
> check
> + * the status of multiple cookies without re-checking hardware state.
> + */
> +static inline enum dma_status dma_async_memcpy_complete(struct
> dma_chan *chan,
> + dma_cookie_t cookie, dma_cookie_t *last, dma_cookie_t *used)
> +{
> + return chan->device->device_memcpy_complete(chan, cookie, last,
> used);
> +}
> +
> +/**
> + * dma_async_is_complete - test a cookie against chan state
> + * @cookie: transaction identifier to test status of
> + * @last_complete: last know completed transaction
> + * @last_used: last cookie value handed out
> + *
> + * dma_async_is_complete() is used in dma_async_memcpy_complete()
> + * the test logic is seperated for lightweight testing of multiple
> cookies
> + */
> +static inline enum dma_status dma_async_is_complete(dma_cookie_t
> cookie,
> + dma_cookie_t last_complete, dma_cookie_t last_used)
> +{
> + if (last_complete <= last_used) {
> + if ((cookie <= last_complete) || (cookie > last_used))
> + return DMA_SUCCESS;
> + } else {
> + if ((cookie <= last_complete) && (cookie > last_used))
> + return DMA_SUCCESS;
> + }
> + return DMA_IN_PROGRESS;
> +}
> +
> +
> +/* --- DMA device --- */
> +
> +int dma_async_device_register(struct dma_device *device);
> +void dma_async_device_unregister(struct dma_device *device);
> +
> +#endif /* CONFIG_DMA_ENGINE */
> +#endif /* DMAENGINE_H */
>
> -
> 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: [git patches] net driver fixes
From: Linus Torvalds @ 2006-03-17 0:42 UTC (permalink / raw)
To: Jeff Garzik
Cc: Andrew Morton, netdev, Linux Kernel Mailing List, Scott Bardone
In-Reply-To: <20060317003041.GA28029@havoc.gtf.org>
On Thu, 16 Mar 2006, Jeff Garzik wrote:
>
> Please pull from 'upstream-fixes' branch of
> master.kernel.org:/pub/scm/linux/kernel/git/jgarzik/netdev-2.6.git
The commit comments for the Chelsio driver fix are a bit unfortunate.
The array clearly _does_ have three elements, it's just that the code used
to access the fourth (that didn't exist), and now it accesses the third.
So when the commit says "The array should contain 2 elements" it's just
being really confused.
Of course, using an array index of "cmdQ_restarted[2]" without any
explanation for why it's index 2, the bug was inevitable. Maybe a symbolic
value for the magic array indices?
Linus
^ permalink raw reply
* [git patches] net driver fixes
From: Jeff Garzik @ 2006-03-17 0:30 UTC (permalink / raw)
To: Andrew Morton, Linus Torvalds; +Cc: netdev, linux-kernel
Please pull from 'upstream-fixes' branch of
master.kernel.org:/pub/scm/linux/kernel/git/jgarzik/netdev-2.6.git
to receive the following updates:
drivers/net/chelsio/sge.c | 6 +++---
drivers/net/e100.c | 5 ++++-
drivers/net/e1000/e1000_main.c | 8 ++++----
net/ieee80211/ieee80211_crypt_ccmp.c | 2 +-
net/ieee80211/ieee80211_rx.c | 4 ++--
5 files changed, 14 insertions(+), 11 deletions(-)
David S. Miller:
e1000 endianness bugs
Hong Liu:
ieee80211: Fix QoS is not active problem
Jesse Brandeburg:
e100: fix eeh on pseries during ethtool -t
Scott Bardone:
[netdrvr] fix array overflows in Chelsio driver
Zhu Yi:
ieee80211: Fix CCMP decryption problem when QoS is enabled
diff --git a/drivers/net/chelsio/sge.c b/drivers/net/chelsio/sge.c
index 2c5b849..30ff8ea 100644
--- a/drivers/net/chelsio/sge.c
+++ b/drivers/net/chelsio/sge.c
@@ -1021,7 +1021,7 @@ static void restart_tx_queues(struct sge
if (test_and_clear_bit(nd->if_port,
&sge->stopped_tx_queues) &&
netif_running(nd)) {
- sge->stats.cmdQ_restarted[3]++;
+ sge->stats.cmdQ_restarted[2]++;
netif_wake_queue(nd);
}
}
@@ -1350,7 +1350,7 @@ static int t1_sge_tx(struct sk_buff *skb
if (unlikely(credits < count)) {
netif_stop_queue(dev);
set_bit(dev->if_port, &sge->stopped_tx_queues);
- sge->stats.cmdQ_full[3]++;
+ sge->stats.cmdQ_full[2]++;
spin_unlock(&q->lock);
if (!netif_queue_stopped(dev))
CH_ERR("%s: Tx ring full while queue awake!\n",
@@ -1358,7 +1358,7 @@ static int t1_sge_tx(struct sk_buff *skb
return NETDEV_TX_BUSY;
}
if (unlikely(credits - count < q->stop_thres)) {
- sge->stats.cmdQ_full[3]++;
+ sge->stats.cmdQ_full[2]++;
netif_stop_queue(dev);
set_bit(dev->if_port, &sge->stopped_tx_queues);
}
diff --git a/drivers/net/e100.c b/drivers/net/e100.c
index 24253c8..f57a85f 100644
--- a/drivers/net/e100.c
+++ b/drivers/net/e100.c
@@ -2154,6 +2154,9 @@ static int e100_loopback_test(struct nic
msleep(10);
+ pci_dma_sync_single_for_cpu(nic->pdev, nic->rx_to_clean->dma_addr,
+ RFD_BUF_LEN, PCI_DMA_FROMDEVICE);
+
if(memcmp(nic->rx_to_clean->skb->data + sizeof(struct rfd),
skb->data, ETH_DATA_LEN))
err = -EAGAIN;
@@ -2161,8 +2164,8 @@ static int e100_loopback_test(struct nic
err_loopback_none:
mdio_write(nic->netdev, nic->mii.phy_id, MII_BMCR, 0);
nic->loopback = lb_none;
- e100_hw_init(nic);
e100_clean_cbs(nic);
+ e100_hw_reset(nic);
err_clean_rx:
e100_rx_clean_list(nic);
return err;
diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index 4c4db96..84dcca3 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -3710,7 +3710,7 @@ e1000_clean_rx_irq(struct e1000_adapter
e1000_rx_checksum(adapter,
(uint32_t)(status) |
((uint32_t)(rx_desc->errors) << 24),
- rx_desc->csum, skb);
+ le16_to_cpu(rx_desc->csum), skb);
skb->protocol = eth_type_trans(skb, netdev);
#ifdef CONFIG_E1000_NAPI
@@ -3854,11 +3854,11 @@ e1000_clean_rx_irq_ps(struct e1000_adapt
}
e1000_rx_checksum(adapter, staterr,
- rx_desc->wb.lower.hi_dword.csum_ip.csum, skb);
+ le16_to_cpu(rx_desc->wb.lower.hi_dword.csum_ip.csum), skb);
skb->protocol = eth_type_trans(skb, netdev);
if (likely(rx_desc->wb.upper.header_status &
- E1000_RXDPS_HDRSTAT_HDRSP))
+ cpu_to_le16(E1000_RXDPS_HDRSTAT_HDRSP)))
adapter->rx_hdr_split++;
#ifdef CONFIG_E1000_NAPI
if (unlikely(adapter->vlgrp && (staterr & E1000_RXD_STAT_VP))) {
@@ -3884,7 +3884,7 @@ e1000_clean_rx_irq_ps(struct e1000_adapt
#endif
next_desc:
- rx_desc->wb.middle.status_error &= ~0xFF;
+ rx_desc->wb.middle.status_error &= cpu_to_le32(~0xFF);
buffer_info->skb = NULL;
/* return some buffers to hardware, one at a time is too slow */
diff --git a/net/ieee80211/ieee80211_crypt_ccmp.c b/net/ieee80211/ieee80211_crypt_ccmp.c
index 4702217..3840d19 100644
--- a/net/ieee80211/ieee80211_crypt_ccmp.c
+++ b/net/ieee80211/ieee80211_crypt_ccmp.c
@@ -131,7 +131,7 @@ static void ccmp_init_blocks(struct cryp
a4_included = ((fc & (IEEE80211_FCTL_TODS | IEEE80211_FCTL_FROMDS)) ==
(IEEE80211_FCTL_TODS | IEEE80211_FCTL_FROMDS));
qc_included = ((WLAN_FC_GET_TYPE(fc) == IEEE80211_FTYPE_DATA) &&
- (WLAN_FC_GET_STYPE(fc) & 0x08));
+ (WLAN_FC_GET_STYPE(fc) & IEEE80211_STYPE_QOS_DATA));
aad_len = 22;
if (a4_included)
aad_len += 6;
diff --git a/net/ieee80211/ieee80211_rx.c b/net/ieee80211/ieee80211_rx.c
index b410ab8..7ac6a71 100644
--- a/net/ieee80211/ieee80211_rx.c
+++ b/net/ieee80211/ieee80211_rx.c
@@ -1417,10 +1417,10 @@ static void ieee80211_process_probe_resp
if (is_beacon(beacon->header.frame_ctl)) {
if (ieee->handle_beacon != NULL)
- ieee->handle_beacon(dev, beacon, &network);
+ ieee->handle_beacon(dev, beacon, target);
} else {
if (ieee->handle_probe_response != NULL)
- ieee->handle_probe_response(dev, beacon, &network);
+ ieee->handle_probe_response(dev, beacon, target);
}
}
^ permalink raw reply related
* Re: drivers/net/chelsio/sge.c: two array overflows
From: Jeff Garzik @ 2006-03-17 0:21 UTC (permalink / raw)
To: Scott Bardone; +Cc: Adrian Bunk, maintainers, netdev, linux-kernel
In-Reply-To: <4415C87B.90107@chelsio.com>
Scott Bardone wrote:
> Adrian,
>
> This is a bug. The array should contain 2 elements.
>
> Attached is a patch which fixes it.
> Thanks.
>
> Signed-off-by: Scott Bardone <sbardone@chelsio.com>
applied. please avoid attachments and use a proper patch description in
the future. I had to hand-edit and hand-apply your patch.
See http://linux.yyz.us/patch-format.html for more info, or use git.
Jeff
^ permalink raw reply
* Re: [PATCH]: e1000 endianness bugs
From: Jeff Garzik @ 2006-03-17 0:17 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, linux-kernel, cramerj, john.ronciak
In-Reply-To: <20060315.142628.28661597.davem@davemloft.net>
David S. Miller wrote:
> return -E_NO_BIG_ENDIAN_TESTING;
>
> [E1000]: Fix 4 missed endianness conversions on RX descriptor fields.
>
> Signed-off-by: David S. Miller <davem@davemloft.net>
applied
^ permalink raw reply
* Re: [PATCH] TC: bug fixes to the "sample" clause
From: Russell Stuart @ 2006-03-16 23:24 UTC (permalink / raw)
To: hadi; +Cc: netdev, lartc
In-Reply-To: <1142517116.5417.137.camel@jzny2>
On Thu, 2006-03-16 at 08:51 -0500, jamal wrote:
> > BTW, in this example, the new hash I suggested would be as good
> > as the 2.6 case.
> >
>
> Yes, if you used 256 buckets per hash table ;->
No - you haven't understood what the new algorithm does.
It will get the same performance of the 2.6 version with
the same memory. In fact for all cases where the number
of bits used is <= 8, ie is _identical_ to the 2.6
algorithm.
> Wait till you start getting into V6. Or try
It is odd that you keep mention IPv6. The IPv6 header
has no fields that are less that 8 bits, and there are
none that are not aligned on a 8 bit boundary. In fact
even within the IPv6 addresses, there are no sub-fields
less that 8 bits. It would be a happy hunting ground for
the 2.4 algorithm.
> building an effective fast lookup scheme for a five tuple lookup using
> u32; five tuples being {src/dst IP, IP protocol, src/dst port}
> So in simple setups where you say dont exceed a few hundred hash tables,
> and a few hundred to thousand filters, the old hash was fine.
> > Now lets take the case that we hashing a number of bytes with
> > a 256 divisor (my case). If these bytes contain truly random
> > values, then again 2.4 and 2.6 will be the same.
>
> But they are not.
Well, this is the crux of the matter, isn't it? You are
apparently used to looking up well known patterns - probably
ones you determine. As such, you can arrange these in nice
logical ranges.
Guess what? The very thing that started this off was me
looking up TCP & UDP port numbers. I didn't determine those
port numbers. They are all over the shop - for me they are
effectively random data. And they are sparse too, as in
they occupy 16 bits. The whole point of the rant that
followed was to explain to you why in a case like that the
2.6 hash runs substantially slower that the 2.4 one. Whats
more, it can't be fixed by simply trading off more memory.
But you seem to be burying you head in the sand and saying
"such data sets don't exist". Well they do. And I gave
you a real life one.
Here is another one: I have contemplated at times giving
priority to Australian traffic. So I went and found myself
a list of Australian IP addresses. Being allocated by some
central authority, I expected some nice ordered data set.
How naive. There are some 400 subnets, and after trying
to find some pattern I gave up after an hour. Another
random dataset. The 2.4 algorithm will handle that well.
When you changed the hash algorithm, you optimised it for
your particular world - at the expense of people who have
data sets like mime. Note that users of 2.4 with your
dataset have a way out - waste a bit of memory and it will
run just as fast. With a large dataset and 2.6 there is
no way out. You are probably doubting this as you are it -
I explain why below.
> > The 2.4 XOR's
> > the two values together. XOR has the property that it "adds"
> > the randomness of the bits together, unless they are correlated.
> > So if you take two partially random bits, and XOR them together,
> > then the resulting bit will be more random that the original two
> > bits. An illustration of this from crypto is a stream cypher
> > like rc4. rc4 effectively produces a random stream of bits.
> > To use rc4, you XOR your plain text with this random stream.
> > Even though your plain text is highly non-random, the cypher
> > text is at least as random as the rc4 stream - and thus looks
> > like gibberish. Anyway, the end result for 2.4 is that if you
> > XOR two perfectly random bytes, the result is a perfectly random
> > byte. So for random data 2.6 and 2.4 are the same.
> >
>
> To use a term which makes sense up here, you are treading on thin ice
> now ;-> You dont wanna skate on this river ;->
>
> Randomness has nothing to do with this. It doesnt matter whether
> random traffic patterns arrive at all.
> - assume that in 8 bits of data, 6 bits provide the most entropy.
> - assume 256 hosts (just to cover the range of the 8 bits)
>
> a) If i built a hash table with 256 buckets, i can guarantee that
> i will find any host using either scheme in 4 lookups.
> Except 75% of the buckets will not be used by either.
You miss one point. While you can guarantee it will
happen in 4 lookups, 2.4 averages slightly more than
1 lookup it if hashes the entire value in one hit.
In 2.6, you are stuck with your 4 lookups, as you say.
> b) If i built a hash table with 128 buckets, I can guarantee i will
> find any host in 4 lookups with the new scheme but it will take
> a best case of 8 lookups with the old scheme.
> Except 50% of the buckets will not be used by the new scheme and
> 75% not be used by the old scheme.
>
> c) if i built a hash table with 64 buckets, I can guarantee that i will
> find any host in 4 lookups with the new scheme and 16 in the old scheme.
> 100% of the buckets will be used by the new scheme; only 25% will be
> used by the old scheme.
>
> d) If i built a hash table of 32 buckets, I can guarantee that i will
> find any host in 8 lookups with new scheme and _32_ with old.
> 100% used by the new scheme and 25% by old
>
> See the pattern? But this is not the worst part. The nasty part
> is if i built a newer level of hash tables so i can reduce the lookups,
> it totally reduces my effectiveness; i need to figure out which buckets
> are being hit etc.
The pattern is based on the (unstated) premise that you
are dropping the least significant bits in the byte in
your hashing function. Perhaps you are assuming you know
where these random bits live in the address. I wasn't
so lucky with my dataset. However lets go with the original
premise - the data is truly random, and you don't know
where bits are. When you drop the least significant bits
2.4 behaves badly. But since the data is random, you are
better off dropping the upper bits when using 2.4. If
you do that then the 2.4 and 2.6 hashes perform
identically when used as you describe above.
In other words, the example didn't show what you intended
it to show. It showed that if you do the optimal thing
for 2.6 and use a tree of hash tables then 2.4 and 2.6 can
be made to perform identically. If you do the optimal
thing for 2.4, then on average 2.4 can be made to run
almost 4 times faster than 2.6, on average.
> You have not convinced me that, in the case of multibyte, the old one is
> good for anything other than the one example you had with a fixed mask
> and fixed number of buckets.
> I hope you see the value of varying the parameters i mentioned now (it
> should be very clear on the hash bucket count and mask i hope).
> Sorry, but a lot more work is needed and you seem to be in a rush to get
> there ;->
As you can probably gather, all we seem to of done here
is concluded that there are two sorts of data sets -
those that are nicely behaved like yours, and those that
aren't - like mine. You seem to be denying mine exist,
which is a pity. 2.6 works well for yours. 2.4 works
well for mine.
Judging from these initial comments:
russell: BTW, in this example, the new hash I suggested
would be as good as the 2.6 case.
jamal: Yes, if you used 256 buckets per hash table.
I don't seem to have explained the new algorithm very well.
Let me try again. There is nothing inherently new about
it. It is just a combination of Alexy's 2.4 algorithm,
and your 2.6 version:
2.4 Algorithm: 2.6 algorithm: New Algorithm:
-------------- -------------- --------------
hash = (hash>>16)^hash; hash = hash>>shift; hash = hash>>shift;
hash = (hash>>8)^hash; return hash & 0xff; hash = (hash>>16)^hash;
return hash & 0xff; hash = (hash>>8)^hash;
return hash & 0xff;
How the new one performs:
(a) For all fields <= 8 bits, it is identical to 2.6.
(b) For all fields aligned on a 8 bit boundary, it is
identical to 2.4.
(c) Importantly, on a single 8 bit aligned value, it is
identical to 2.4 and 2.6: it is the identity function.
This keeps the cut & pasters happy.
Now everyone wins: those with nice orderly datasets like
yours, and those with sparse, random data sets like mine.
Why not change over to it?
^ permalink raw reply
* [iproute2] IPoIB link layer address bug
From: James Lentini @ 2006-03-16 22:24 UTC (permalink / raw)
To: shemminger; +Cc: netdev, openib-general, lartc
The ip(8) command has a bug when dealing with IPoIB link layer
addresses. Specifically it does not correctly handle the addition of
new entries in the neighbor/arp table. For example, this command will
fail:
ip neigh add 192.168.0.138 lladdr 00:00:04:04:fe:80:00:00:00:00:00:00:00:01:73:00:00:00:8a:91 nud permanent dev ib0
An IPoIB link layer address is 20-bytes (see
http://www.ietf.org/internet-drafts/draft-ietf-ipoib-ip-over-infiniband-09.txt,
section 9.1.1).
The command line parsing code expects link layer addresses to be a
maximum of 16-bytes. Addresses over 16-bytes are truncated.
This patch (against the iproute2 cvs repository) fixes the problem:
============================================
--- iproute2/ip/ipneigh.c.orig 2005-09-01 15:21:50.000000000 -0400
+++ iproute2/ip/ipneigh.c 2006-03-16 17:03:41.339759000 -0500
@@ -165,7 +165,7 @@ static int ipneigh_modify(int cmd, int f
addattr_l(&req.n, sizeof(req), NDA_DST, &dst.data, dst.bytelen);
if (lla && strcmp(lla, "null")) {
- char llabuf[16];
+ char llabuf[20];
int l;
l = ll_addr_a2n(llabuf, sizeof(llabuf), lla);
============================================
P.S. - I've found a similar issue with the arp command, see
http://openib.org/pipermail/openib-general/2006-March/018270.html
^ permalink raw reply
* RE: Router stops routing after changing MAC Address
From: Greg Scott @ 2006-03-16 18:32 UTC (permalink / raw)
To: Stephen Hemminger, Chris Wedgwood
Cc: Chuck Ebbert, linux-kernel, David S. Miller, netdev, Bart Samwel,
Alan Cox, Simon Mackinlay
I wonder if they would be more open to accepting that patch now?
- Greg Scott
-----Original Message-----
From: Stephen Hemminger [mailto:shemminger@osdl.org]
Sent: Thursday, March 16, 2006 11:55 AM
To: Chris Wedgwood
Cc: Greg Scott; Chuck Ebbert; linux-kernel; David S. Miller;
netdev@vger.kernel.org; Bart Samwel; Alan Cox; Simon Mackinlay
Subject: Re: Router stops routing after changing MAC Address
On Thu, 16 Mar 2006 08:07:43 -0800
Chris Wedgwood <cw@f00f.org> wrote:
> On Mon, Mar 13, 2006 at 10:00:41AM -0800, Stephen Hemminger wrote:
>
> > There still is a bug in the 3c59x driver. It doesn't include any
> > code to handle changing the mac address. It will work if you take
> > the device down, change address, then bring it up. But you shouldn't
> > have to do that.
>
> I sent a patch do to this probably a year or two back and it was
> rejected (by akpm if I recall) because of the argument that you could
> and should take it down, change the MAC and bring it back up.
>
> Is this no longer a requirement?
No. most drivers allow changes on the fly.
^ permalink raw reply
* Re: Router stops routing after changing MAC Address
From: Stephen Hemminger @ 2006-03-16 17:55 UTC (permalink / raw)
To: Chris Wedgwood
Cc: Greg Scott, Chuck Ebbert, linux-kernel, David S. Miller, netdev,
Bart Samwel, Alan Cox, Simon Mackinlay
In-Reply-To: <20060316160743.GA13035@taniwha.stupidest.org>
On Thu, 16 Mar 2006 08:07:43 -0800
Chris Wedgwood <cw@f00f.org> wrote:
> On Mon, Mar 13, 2006 at 10:00:41AM -0800, Stephen Hemminger wrote:
>
> > There still is a bug in the 3c59x driver. It doesn't include any
> > code to handle changing the mac address. It will work if you take
> > the device down, change address, then bring it up. But you shouldn't
> > have to do that.
>
> I sent a patch do to this probably a year or two back and it was
> rejected (by akpm if I recall) because of the argument that you could
> and should take it down, change the MAC and bring it back up.
>
> Is this no longer a requirement?
No. most drivers allow changes on the fly.
^ permalink raw reply
* Re: Router stops routing after changing MAC Address
From: Chris Wedgwood @ 2006-03-16 16:07 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Greg Scott, Chuck Ebbert, linux-kernel, David S. Miller, netdev,
Bart Samwel, Alan Cox, Simon Mackinlay
In-Reply-To: <20060313100041.212cee08@localhost.localdomain>
On Mon, Mar 13, 2006 at 10:00:41AM -0800, Stephen Hemminger wrote:
> There still is a bug in the 3c59x driver. It doesn't include any
> code to handle changing the mac address. It will work if you take
> the device down, change address, then bring it up. But you shouldn't
> have to do that.
I sent a patch do to this probably a year or two back and it was
rejected (by akpm if I recall) because of the argument that you could
and should take it down, change the MAC and bring it back up.
Is this no longer a requirement?
^ permalink raw reply
* Re: [patch 1/4] natsemi: Add support for using MII port with no PHY
From: Mark Brown @ 2006-03-16 9:26 UTC (permalink / raw)
To: Andrew Morton; +Cc: thockin, jgarzik, netdev, linux-kernel
In-Reply-To: <20060316010902.57e3a98b.akpm@osdl.org>
On Thu, Mar 16, 2006 at 01:09:02AM -0800, Andrew Morton wrote:
> Mark Brown <broonie@sirena.org.uk> wrote:
> > + if (np->ignore_phy && (ecmd->autoneg == AUTONEG_ENABLE ||
> > + ecmd->port == PORT_INTERNAL)) {
> What's PORT_INTERNAL? ethtool doesn't appear to define that.
It should be PORT_TP, sorry:
This patch provides a module option which configures the natsemi driver
to use the external MII port on the chip but ignore any PHYs that may be
attached to it. The link state will be left as it was when the driver
started and can be configured via ethtool. Any PHYs that are present
can be accessed via the MII ioctl()s.
This is useful for systems where the device is connected without a PHY
or where either information or actions outside the scope of the driver
are required in order to use the PHYs.
Signed-Off-By: Mark Brown <broonie@sirena.org.uk>
Index: natsemi-queue/drivers/net/natsemi.c
===================================================================
--- natsemi-queue.orig/drivers/net/natsemi.c 2006-02-25 13:38:34.000000000 +0000
+++ natsemi-queue/drivers/net/natsemi.c 2006-02-25 13:50:51.000000000 +0000
@@ -259,7 +259,7 @@ MODULE_PARM_DESC(debug, "DP8381x default
MODULE_PARM_DESC(rx_copybreak,
"DP8381x copy breakpoint for copy-only-tiny-frames");
MODULE_PARM_DESC(options,
- "DP8381x: Bits 0-3: media type, bit 17: full duplex");
+ "DP8381x: Bits 0-3: media type, bit 17: full duplex, bit 18: ignore PHY");
MODULE_PARM_DESC(full_duplex, "DP8381x full duplex setting(s) (1)");
/*
@@ -690,6 +690,8 @@ struct netdev_private {
u32 intr_status;
/* Do not touch the nic registers */
int hands_off;
+ /* Don't pay attention to the reported link state. */
+ int ignore_phy;
/* external phy that is used: only valid if dev->if_port != PORT_TP */
int mii;
int phy_addr_external;
@@ -891,7 +893,19 @@ static int __devinit natsemi_probe1 (str
np->hands_off = 0;
np->intr_status = 0;
+ option = find_cnt < MAX_UNITS ? options[find_cnt] : 0;
+ if (dev->mem_start)
+ option = dev->mem_start;
+
+ /* Ignore the PHY status? */
+ if (option & 0x400) {
+ np->ignore_phy = 1;
+ } else {
+ np->ignore_phy = 0;
+ }
+
/* Initial port:
+ * - If configured to ignore the PHY set up for external.
* - If the nic was configured to use an external phy and if find_mii
* finds a phy: use external port, first phy that replies.
* - Otherwise: internal port.
@@ -899,7 +913,7 @@ static int __devinit natsemi_probe1 (str
* The address would be used to access a phy over the mii bus, but
* the internal phy is accessed through mapped registers.
*/
- if (readl(ioaddr + ChipConfig) & CfgExtPhy)
+ if (np->ignore_phy || readl(ioaddr + ChipConfig) & CfgExtPhy)
dev->if_port = PORT_MII;
else
dev->if_port = PORT_TP;
@@ -909,7 +923,9 @@ static int __devinit natsemi_probe1 (str
if (dev->if_port != PORT_TP) {
np->phy_addr_external = find_mii(dev);
- if (np->phy_addr_external == PHY_ADDR_NONE) {
+ /* If we're ignoring the PHY it doesn't matter if we can't
+ * find one. */
+ if (!np->ignore_phy && np->phy_addr_external == PHY_ADDR_NONE) {
dev->if_port = PORT_TP;
np->phy_addr_external = PHY_ADDR_INTERNAL;
}
@@ -917,10 +933,6 @@ static int __devinit natsemi_probe1 (str
np->phy_addr_external = PHY_ADDR_INTERNAL;
}
- option = find_cnt < MAX_UNITS ? options[find_cnt] : 0;
- if (dev->mem_start)
- option = dev->mem_start;
-
/* The lower four bits are the media type. */
if (option) {
if (option & 0x200)
@@ -954,7 +966,10 @@ static int __devinit natsemi_probe1 (str
if (mtu)
dev->mtu = mtu;
- netif_carrier_off(dev);
+ if (np->ignore_phy)
+ netif_carrier_on(dev);
+ else
+ netif_carrier_off(dev);
/* get the initial settings from hardware */
tmp = mdio_read(dev, MII_BMCR);
@@ -1002,6 +1017,8 @@ static int __devinit natsemi_probe1 (str
printk("%02x, IRQ %d", dev->dev_addr[i], irq);
if (dev->if_port == PORT_TP)
printk(", port TP.\n");
+ else if (np->ignore_phy)
+ printk(", port MII, ignoring PHY\n");
else
printk(", port MII, phy ad %d.\n", np->phy_addr_external);
}
@@ -1682,42 +1699,44 @@ static void check_link(struct net_device
{
struct netdev_private *np = netdev_priv(dev);
void __iomem * ioaddr = ns_ioaddr(dev);
- int duplex;
+ int duplex = np->full_duplex;
u16 bmsr;
-
- /* The link status field is latched: it remains low after a temporary
- * link failure until it's read. We need the current link status,
- * thus read twice.
- */
- mdio_read(dev, MII_BMSR);
- bmsr = mdio_read(dev, MII_BMSR);
- if (!(bmsr & BMSR_LSTATUS)) {
- if (netif_carrier_ok(dev)) {
+ /* If we're not paying attention to the PHY status then don't check. */
+ if (!np->ignore_phy) {
+ /* The link status field is latched: it remains low
+ * after a temporary link failure until it's read. We
+ * need the current link status, thus read twice.
+ */
+ mdio_read(dev, MII_BMSR);
+ bmsr = mdio_read(dev, MII_BMSR);
+
+ if (!(bmsr & BMSR_LSTATUS)) {
+ if (netif_carrier_ok(dev)) {
+ if (netif_msg_link(np))
+ printk(KERN_NOTICE "%s: link down.\n",
+ dev->name);
+ netif_carrier_off(dev);
+ undo_cable_magic(dev);
+ }
+ return;
+ }
+ if (!netif_carrier_ok(dev)) {
if (netif_msg_link(np))
- printk(KERN_NOTICE "%s: link down.\n",
- dev->name);
- netif_carrier_off(dev);
- undo_cable_magic(dev);
+ printk(KERN_NOTICE "%s: link up.\n", dev->name);
+ netif_carrier_on(dev);
+ do_cable_magic(dev);
}
- return;
- }
- if (!netif_carrier_ok(dev)) {
- if (netif_msg_link(np))
- printk(KERN_NOTICE "%s: link up.\n", dev->name);
- netif_carrier_on(dev);
- do_cable_magic(dev);
- }
- duplex = np->full_duplex;
- if (!duplex) {
- if (bmsr & BMSR_ANEGCOMPLETE) {
- int tmp = mii_nway_result(
- np->advertising & mdio_read(dev, MII_LPA));
- if (tmp == LPA_100FULL || tmp == LPA_10FULL)
+ if (!duplex) {
+ if (bmsr & BMSR_ANEGCOMPLETE) {
+ int tmp = mii_nway_result(
+ np->advertising & mdio_read(dev, MII_LPA));
+ if (tmp == LPA_100FULL || tmp == LPA_10FULL)
+ duplex = 1;
+ } else if (mdio_read(dev, MII_BMCR) & BMCR_FULLDPLX)
duplex = 1;
- } else if (mdio_read(dev, MII_BMCR) & BMCR_FULLDPLX)
- duplex = 1;
+ }
}
/* if duplex is set then bit 28 must be set, too */
@@ -2927,6 +2946,16 @@ static int netdev_set_ecmd(struct net_de
}
/*
+ * If we're ignoring the PHY then autoneg and the internal
+ * transciever are really not going to work so don't let the
+ * user select them.
+ */
+ if (np->ignore_phy && (ecmd->autoneg == AUTONEG_ENABLE ||
+ ecmd->port == PORT_TP)) {
+ return -EINVAL;
+ }
+
+ /*
* maxtxpkt, maxrxpkt: ignored for now.
*
* transceiver:
--
"You grabbed my hand and we fell into it, like a daydream - or a fever."
^ permalink raw reply
* Re: [patch 1/4] natsemi: Add support for using MII port with no PHY
From: Andrew Morton @ 2006-03-16 9:09 UTC (permalink / raw)
To: Mark Brown; +Cc: thockin, jgarzik, netdev, linux-kernel
In-Reply-To: <20060312205303.869316000@mercator.sirena.org.uk>
Mark Brown <broonie@sirena.org.uk> wrote:
>
> + if (np->ignore_phy && (ecmd->autoneg == AUTONEG_ENABLE ||
> + ecmd->port == PORT_INTERNAL)) {
What's PORT_INTERNAL? ethtool doesn't appear to define that.
drivers/net/natsemi.c: In function `netdev_set_ecmd':
drivers/net/natsemi.c:2989: `PORT_INTERNAL' undeclared (first use in this function)
drivers/net/natsemi.c:2989: (Each undeclared identifier is reported only once
drivers/net/natsemi.c:2989: for each function it appears in.)
^ permalink raw reply
* Re: [PATCH] IPv6: Cleanups for net/ipv6/addrconf.c (kzalloc, early exit) v2
From: David S. Miller @ 2006-03-16 8:20 UTC (permalink / raw)
To: ioe-lkml; +Cc: yoshfuji, linux-kernel, netdev
In-Reply-To: <200603102334.27590.ioe-lkml@rameria.de>
From: Ingo Oeser <ioe-lkml@rameria.de>
Date: Fri, 10 Mar 2006 23:34:26 +0100
> Here are some possible (and trivial) cleanups.
> - use kzalloc() where possible
> - invert allocation failure test like
> if (object) {
> /* Rest of function here */
> }
> to
>
> if (object == NULL)
> return NULL;
>
> /* Rest of function here */
>
> Signed-off-by: Ingo Oeser <ioe-lkml@rameria.de>
> Acked-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
Applied, thanks a lot Ingo.
^ permalink raw reply
* Re: [PATCH] Nearly complete kzalloc cleanup for net/ipv6
From: David S. Miller @ 2006-03-16 8:17 UTC (permalink / raw)
To: ioe-lkml; +Cc: yoshfuji, linux-kernel, netdev
In-Reply-To: <200603112136.43553.ioe-lkml@rameria.de>
From: Ingo Oeser <ioe-lkml@rameria.de>
Date: Sat, 11 Mar 2006 21:36:42 +0100
> Stupidly use kzalloc() instead of kmalloc()/memset()
> everywhere where this is possible in net/ipv6/*.c .
>
> Signed-off-by: Ingo Oeser <ioe-lkml@rameria.de>
This patch also looks fine.
Applied, thanks Ingo.
^ permalink raw reply
* Re: [PATCH] IPv6: Cleanup of net/ipv6/reassambly.c
From: David S. Miller @ 2006-03-16 8:14 UTC (permalink / raw)
To: ioe-lkml; +Cc: yoshfuji, netdev, linux-kernel
In-Reply-To: <200603120049.49294.ioe-lkml@rameria.de>
From: Ingo Oeser <ioe-lkml@rameria.de>
Date: Sun, 12 Mar 2006 00:49:48 +0100
> Two minor cleanups:
>
> 1. Using kzalloc() in fraq_alloc_queue()
> saves the memset() in ipv6_frag_create().
>
> 2. Invert sense of if-statements to streamline code.
> Inverts the comment, too.
>
> Signed-off-by: Ingo Oeser <ioe-lkml@rameria.de>
This patch looks great, applied.
Thanks a lot Ingo.
^ permalink raw reply
* Re: [PATCH]: e1000 endianness bugs
From: Christoph Hellwig @ 2006-03-16 7:31 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, linux-kernel, jeff, cramerj, john.ronciak
In-Reply-To: <20060315.142628.28661597.davem@davemloft.net>
On Wed, Mar 15, 2006 at 02:26:28PM -0800, David S. Miller wrote:
>
> return -E_NO_BIG_ENDIAN_TESTING;
>
> [E1000]: Fix 4 missed endianness conversions on RX descriptor fields.
Could the e1000 maintainers please add endianess annotations so that
sparse will catch such things in the future?
^ permalink raw reply
* Re: [PATCH] TC: bug fixes to the "sample" clause
From: Russell Stuart @ 2006-03-16 1:43 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: lartc, hadi, netdev
In-Reply-To: <20060315165757.44bf1548@localhost.localdomain>
<snip>Much discussion bashing this issue to death.</snip>
(sorry, jamal - this one is CC'ed to lartc.)
Here is are revised versions of the 2 as yet unapplied
patches.
PATCH 1
=======
[Has been applied.]
PATCH 2
=======
In tc, the u32 sample clause uses the 2.4 hashing algorithm.
The hashing algorithm used by the kernel changed in 2.6,
consequently "sample" hasn't work since then.
This patch makes the sample clause work 2.6 only. This is
different from the prior version of the patch, in that it
made it work with 2.4 and 2.6:
diff -Nur iproute-20051007.keep/tc/f_u32.c iproute-20051007/tc/f_u32.c
--- iproute-20051007.keep/tc/f_u32.c 2006-03-14 12:28:17.000000000 +1000
+++ iproute-20051007/tc/f_u32.c 2006-03-16 11:08:25.000000000 +1000
@@ -888,8 +888,18 @@
return -1;
}
hash = sel2.sel.keys[0].val&sel2.sel.keys[0].mask;
+#if 0
+ /* 2.2 .. 2.4 hashing algorithm */
hash ^= hash>>16;
hash ^= hash>>8;
+#else
+ /* 2.5 onwards */
+ __u32 mask = sel2.sel.keys[0].mask;
+ while (mask && !(mask & 1)) {
+ mask >>= 1;
+ hash >>= 1;
+ }
+#endif
htid = ((hash<<12)&0xFF000)|(htid&0xFFF00000);
sample_ok = 1;
continue;
PATCH 3
=======
"tc" does not allow you to specify the divisor for the
"sample" clause, it always assumes a divisor of 256.
If the divisor isn't 256, (ie it is something less),
the kernel will usually whinge because the bucket given
to it by "tc" is typically too big.
This patch adds a "divisor" option to tc's "sample"
clause. This is identical to the previous version of
the patch, other than it now applies correctly after
the revised PATCH 3.
diff -Nur iproute-20051007.keep/tc/f_u32.c iproute-20051007/tc/f_u32.c
--- iproute-20051007.keep/tc/f_u32.c 2006-03-16 11:27:17.000000000 +1000
+++ iproute-20051007/tc/f_u32.c 2006-03-16 11:29:56.000000000 +1000
@@ -34,7 +34,7 @@
fprintf(stderr, "or u32 divisor DIVISOR\n");
fprintf(stderr, "\n");
fprintf(stderr, "Where: SELECTOR := SAMPLE SAMPLE ...\n");
- fprintf(stderr, " SAMPLE := { ip | ip6 | udp | tcp | icmp | u{32|16|8} | mark } SAMPLE_ARGS\n");
+ fprintf(stderr, " SAMPLE := { ip | ip6 | udp | tcp | icmp | u{32|16|8} | mark } SAMPLE_ARGS [divisor DIVISOR]\n");
fprintf(stderr, " FILTERID := X:Y:Z\n");
}
@@ -834,7 +834,7 @@
unsigned divisor;
NEXT_ARG();
if (get_unsigned(&divisor, *argv, 0) || divisor == 0 ||
- divisor > 0x100) {
+ divisor > 0x100 || (divisor - 1 & divisor)) {
fprintf(stderr, "Illegal \"divisor\"\n");
return -1;
}
@@ -874,6 +874,7 @@
htid = (handle&0xFFFFF000);
} else if (strcmp(*argv, "sample") == 0) {
__u32 hash;
+ unsigned divisor = 0x100;
struct {
struct tc_u32_sel sel;
struct tc_u32_key keys[4];
@@ -888,6 +889,15 @@
fprintf(stderr, "\"sample\" must contain exactly ONE key.\n");
return -1;
}
+ if (*argv != 0 && strcmp(*argv, "divisor") == 0) {
+ NEXT_ARG();
+ if (get_unsigned(&divisor, *argv, 0) || divisor == 0 ||
+ divisor > 0x100 || (divisor - 1 & divisor)) {
+ fprintf(stderr, "Illegal sample \"divisor\"\n");
+ return -1;
+ }
+ NEXT_ARG();
+ }
hash = sel2.sel.keys[0].val&sel2.sel.keys[0].mask;
#if 0
/* 2.2 .. 2.4 hashing algorithm */
@@ -901,7 +911,7 @@
hash >>= 1;
}
#endif
- htid = ((hash<<12)&0xFF000)|(htid&0xFFF00000);
+ htid = ((hash%divisor)<<12)|(htid&0xFFF00000);
sample_ok = 1;
continue;
} else if (strcmp(*argv, "indev") == 0) {
^ permalink raw reply
* Re: [PATCH]: e1000 endianness bugs
From: David S. Miller @ 2006-03-15 23:40 UTC (permalink / raw)
To: jesse.brandeburg
Cc: netdev, linux-kernel, jeff, cramerj, john.ronciak,
jesse.brandeburg
In-Reply-To: <4807377b0603151533i6a693d99ycdab71fe0a21dc4c@mail.gmail.com>
From: "Jesse Brandeburg" <jesse.brandeburg@gmail.com>
Date: Wed, 15 Mar 2006 15:33:43 -0800
> Yep, those look like bugs to me, thanks and congratulations, you're
> the first person to test our PCI Express adapters in a big endian
> system (they haven't been available before, and we don't have one,
> yet)
It was onboard a Niagara T2000 system.
^ permalink raw reply
* Re: [PATCH]: e1000 endianness bugs
From: Jesse Brandeburg @ 2006-03-15 23:33 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, linux-kernel, jeff, cramerj, john.ronciak,
Jesse Brandeburg
In-Reply-To: <20060315.142628.28661597.davem@davemloft.net>
On 3/15/06, David S. Miller <davem@davemloft.net> wrote:
>
> return -E_NO_BIG_ENDIAN_TESTING;
>
> [E1000]: Fix 4 missed endianness conversions on RX descriptor fields.
>
> Signed-off-by: David S. Miller <davem@davemloft.net>
Yep, those look like bugs to me, thanks and congratulations, you're
the first person to test our PCI Express adapters in a big endian
system (they haven't been available before, and we don't have one,
yet)
Acked-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
^ permalink raw reply
* [PATCH]: e1000 endianness bugs
From: David S. Miller @ 2006-03-15 22:26 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, jeff, cramerj, john.ronciak
return -E_NO_BIG_ENDIAN_TESTING;
[E1000]: Fix 4 missed endianness conversions on RX descriptor fields.
Signed-off-by: David S. Miller <davem@davemloft.net>
diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index 5b7d0f4..1d91117 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -3710,7 +3710,7 @@ e1000_clean_rx_irq(struct e1000_adapter
e1000_rx_checksum(adapter,
(uint32_t)(status) |
((uint32_t)(rx_desc->errors) << 24),
- rx_desc->csum, skb);
+ le16_to_cpu(rx_desc->csum), skb);
skb->protocol = eth_type_trans(skb, netdev);
#ifdef CONFIG_E1000_NAPI
@@ -3854,11 +3854,11 @@ e1000_clean_rx_irq_ps(struct e1000_adapt
}
e1000_rx_checksum(adapter, staterr,
- rx_desc->wb.lower.hi_dword.csum_ip.csum, skb);
+ le16_to_cpu(rx_desc->wb.lower.hi_dword.csum_ip.csum), skb);
skb->protocol = eth_type_trans(skb, netdev);
if (likely(rx_desc->wb.upper.header_status &
- E1000_RXDPS_HDRSTAT_HDRSP))
+ cpu_to_le16(E1000_RXDPS_HDRSTAT_HDRSP)))
adapter->rx_hdr_split++;
#ifdef CONFIG_E1000_NAPI
if (unlikely(adapter->vlgrp && (staterr & E1000_RXD_STAT_VP))) {
@@ -3884,7 +3884,7 @@ e1000_clean_rx_irq_ps(struct e1000_adapt
#endif
next_desc:
- rx_desc->wb.middle.status_error &= ~0xFF;
+ rx_desc->wb.middle.status_error &= cpu_to_le32(~0xFF);
buffer_info->skb = NULL;
/* return some buffers to hardware, one at a time is too slow */
^ permalink raw reply related
* [2.6 patch] ieee80211_wx.c: remove dead code
From: Adrian Bunk @ 2006-03-15 16:40 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel
Since sec->key_sizes[] is an u8, len can't be < 0.
Spotted by the Coverity checker.
Signed-off-by: Adrian Bunk <bunk@stusta.de>
--- linux-2.6.16-rc6-mm1-full/net/ieee80211/ieee80211_wx.c.old 2006-03-14 03:01:43.000000000 +0100
+++ linux-2.6.16-rc6-mm1-full/net/ieee80211/ieee80211_wx.c 2006-03-14 03:02:02.000000000 +0100
@@ -505,7 +505,7 @@ int ieee80211_wx_get_encode(struct ieee8
len = sec->key_sizes[key];
memcpy(keybuf, sec->keys[key], len);
- erq->length = (len >= 0 ? len : 0);
+ erq->length = len;
erq->flags |= IW_ENCODE_ENABLED;
if (ieee->open_wep)
^ permalink raw reply
* Re: [2.6 patch] hostap_{pci,plx}.c: fix memory leaks
From: Adrian Bunk @ 2006-03-15 16:14 UTC (permalink / raw)
To: hostap, netdev, linux-kernel; +Cc: Herbert Xu
In-Reply-To: <20060315031625.GE9384@jm.kir.nu>
On Tue, Mar 14, 2006 at 07:16:25PM -0800, Jouni Malinen wrote:
> On Mon, Mar 13, 2006 at 11:28:41PM +0100, Adrian Bunk wrote:
> > This patch fixes two memotry leaks spotted by the Coverity checker.
>
> Thanks. I'll make a bit different patch to resolve this and related PCI
> "leaks" in one change. I'm going through the Coverity reports for Host
> AP driver, so I'll include other fixes in a patch set, too.
Thanks (I assume you've seem Herbert's comment why my patch was wrong).
> Jouni Malinen PGP id EFC895FA
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
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