* Re: [PATCH net-next-2.6 1/2] be2net: Adding an option to use INTx instead of MSI-X
From: Michael Ellerman @ 2010-11-03 12:45 UTC (permalink / raw)
To: Matthew Wilcox; +Cc: David Miller, bhutchings, somnath.kotur, netdev, linux-pci
In-Reply-To: <20101030232155.GA14129@parisc-linux.org>
[-- Attachment #1: Type: text/plain, Size: 1127 bytes --]
On Sat, 2010-10-30 at 17:21 -0600, Matthew Wilcox wrote:
> On Tue, Oct 26, 2010 at 05:52:08PM +1100, Michael Ellerman wrote:
> > That horse has really really bolted, it's gawn.
> >
> > I count 26 drivers with "disable MSI/X" parameters. Some even have more
> > than one.
>
> That's 26 patches someone needs to write, then. You can put my Acked-by
> on all of them.
Bah, come on it's hardly the most horrendous sin committed by driver
writers. And removing them risks breaking someone's system, even if they
are clueless, should RTFM etc.
> > I agree it's a mess for users, but it's probably preferable to a
> > non-working driver.
>
> What more drivers need is an automatic detection of a non-working
> interrupt situation, great big warning messages, and fallback to an
> alternate interrupt mechanism. Doing it for one driver, then generalising
> as much of it into the core as possible would be nice.
More detection would be good.
I don't see much potential for generalising it though. Looking at e1000e
and tg3 there is really not much in common except the very basic idea.
cheers
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] r6040: fix multicast operations
From: Shawn Lin @ 2010-11-03 12:51 UTC (permalink / raw)
To: netdev, Marc Leclerc, Albert Chen, David Miller, Florian Fainelli
In-Reply-To: <201010202309.43812.florian@openwrt.org>
On Wed, 2010-10-20 at 23:09 +0200, Florian Fainelli wrote:
> This patch fixes the following issues with the r6040 NIC operating in
> multicast:
>
> 1) When the IFF_ALLMULTI flag is set, we should write 0xffff to the NIC hash
> table registers to make it process multicast traffic
> 2) When the number of multicast address to handle is smaller than MCAST_MAX
> we should use the NIC multicast registers MID1_{L,M,H}.
> 3) The hashing of the address was not correct, due to an invalid substraction
> (15 - (crc & 0x0f)) instead of (crc & 0x0f)
I suggest to modify the comment as follows.
3) The hashing of the address was not correct, due to an invalid
substraction (15 - (crc & 0x0f)) instead of (crc & 0x0f) and an
incorrect crc algorithm (ether_crc_le) instead of (ether_crc).
[...]
The original code I submitted to Florian has some issues mentioned by Ben Hutchings.
This revision fixes these issues and another issue about the sequence of configuring multicast hash table registers.
The correct sequence is to enable multicast function before write values to hash table registers. I have verified it on my platform.
The hash algorithm is provided by hardware designers. I also re-confirmed it with RDC's engineer.
Please let me know if anyone has questions.
The version is for net-next-2.6:
---
diff --git a/drivers/net/r6040.c b/drivers/net/r6040.c
index 0b014c8..e88e171 100644
--- a/drivers/net/r6040.c
+++ b/drivers/net/r6040.c
@@ -69,6 +69,8 @@
/* MAC registers */
#define MCR0 0x00 /* Control register 0 */
+#define PROMISC 0x0020 /* Promiscuous mode */
+#define HASH_EN 0x0100 /* Enable multicast hash table function */
#define MCR1 0x04 /* Control register 1 */
#define MAC_RST 0x0001 /* Reset the MAC */
#define MBCR 0x08 /* Bus control */
@@ -851,77 +853,84 @@ static void r6040_multicast_list(struct net_device *dev)
{
struct r6040_private *lp = netdev_priv(dev);
void __iomem *ioaddr = lp->base;
- u16 *adrp;
- u16 reg;
unsigned long flags;
struct netdev_hw_addr *ha;
int i;
+ u16 hash_table[4] = { 0, };
- /* MAC Address */
- adrp = (u16 *)dev->dev_addr;
- iowrite16(adrp[0], ioaddr + MID_0L);
- iowrite16(adrp[1], ioaddr + MID_0M);
- iowrite16(adrp[2], ioaddr + MID_0H);
-
- /* Promiscous Mode */
spin_lock_irqsave(&lp->lock, flags);
/* Clear AMCP & PROM bits */
- reg = ioread16(ioaddr) & ~0x0120;
+ lp->mcr0 = ioread16(ioaddr + MCR0) & ~(PROMISC | HASH_EN);
+
+ /* Promiscuous Mode */
if (dev->flags & IFF_PROMISC) {
- reg |= 0x0020;
- lp->mcr0 |= 0x0020;
+ lp->mcr0 |= PROMISC;
}
- /* Too many multicast addresses
- * accept all traffic */
- else if ((netdev_mc_count(dev) > MCAST_MAX) ||
- (dev->flags & IFF_ALLMULTI))
- reg |= 0x0020;
-
- iowrite16(reg, ioaddr);
- spin_unlock_irqrestore(&lp->lock, flags);
+ /* Enable multicast hash table function to
+ * receive all multicast packets. */
+ else if (dev->flags & IFF_ALLMULTI) {
+ lp->mcr0 |= HASH_EN;
+
+ for (i = 0; i < MCAST_MAX ; i++) {
+ iowrite16(0, ioaddr + MID_1L + 8 * i);
+ iowrite16(0, ioaddr + MID_1M + 8 * i);
+ iowrite16(0, ioaddr + MID_1H + 8 * i);
+ }
- /* Build the hash table */
- if (netdev_mc_count(dev) > MCAST_MAX) {
- u16 hash_table[4];
+ for (i = 0; i < 4; i++)
+ hash_table[i] = 0xffff;
+ }
+ /* Use internal multicast address registers if the number of
+ * multicast addresses is not greater than MCAST_MAX. */
+ else if (netdev_mc_count(dev) <= MCAST_MAX) {
+ i = 0;
+ netdev_for_each_mc_addr(ha, dev) {
+ u16 *adrp = (u16 *) ha->addr;
+ iowrite16(adrp[0], ioaddr + MID_1L + 8 * i);
+ iowrite16(adrp[1], ioaddr + MID_1M + 8 * i);
+ iowrite16(adrp[2], ioaddr + MID_1H + 8 * i);
+ i++;
+ }
+ while (i < MCAST_MAX) {
+ iowrite16(0, ioaddr + MID_1L + 8 * i);
+ iowrite16(0, ioaddr + MID_1M + 8 * i);
+ iowrite16(0, ioaddr + MID_1H + 8 * i);
+ i++;
+ }
+ }
+ /* Otherwise, Enable multicast hash table function. */
+ else {
u32 crc;
- for (i = 0; i < 4; i++)
- hash_table[i] = 0;
+ lp->mcr0 |= HASH_EN;
- netdev_for_each_mc_addr(ha, dev) {
- char *addrs = ha->addr;
+ for (i = 0; i < MCAST_MAX ; i++) {
+ iowrite16(0, ioaddr + MID_1L + 8 * i);
+ iowrite16(0, ioaddr + MID_1M + 8 * i);
+ iowrite16(0, ioaddr + MID_1H + 8 * i);
+ }
- if (!(*addrs & 1))
- continue;
+ /* Build multicast hash table */
+ netdev_for_each_mc_addr(ha, dev) {
+ u8 *addrs = ha->addr;
- crc = ether_crc_le(6, addrs);
+ crc = ether_crc(ETH_ALEN, addrs);
crc >>= 26;
- hash_table[crc >> 4] |= 1 << (15 - (crc & 0xf));
+ hash_table[crc >> 4] |= 1 << (crc & 0xf);
}
- /* Fill the MAC hash tables with their values */
+ }
+ iowrite16(lp->mcr0, ioaddr + MCR0);
+
+ /* Fill the MAC hash tables with their values */
+ if (lp->mcr0 && HASH_EN) {
iowrite16(hash_table[0], ioaddr + MAR0);
iowrite16(hash_table[1], ioaddr + MAR1);
iowrite16(hash_table[2], ioaddr + MAR2);
iowrite16(hash_table[3], ioaddr + MAR3);
}
- /* Multicast Address 1~4 case */
- i = 0;
- netdev_for_each_mc_addr(ha, dev) {
- if (i >= MCAST_MAX)
- break;
- adrp = (u16 *) ha->addr;
- iowrite16(adrp[0], ioaddr + MID_1L + 8 * i);
- iowrite16(adrp[1], ioaddr + MID_1M + 8 * i);
- iowrite16(adrp[2], ioaddr + MID_1H + 8 * i);
- i++;
- }
- while (i < MCAST_MAX) {
- iowrite16(0xffff, ioaddr + MID_1L + 8 * i);
- iowrite16(0xffff, ioaddr + MID_1M + 8 * i);
- iowrite16(0xffff, ioaddr + MID_1H + 8 * i);
- i++;
- }
+
+ spin_unlock_irqrestore(&lp->lock, flags);
}
static void netdev_get_drvinfo(struct net_device *dev,
---
The version is for 2.6.32.y and 2.6.27.y:
---
diff --git a/drivers/net/r6040.c b/drivers/net/r6040.c
index 9ee9f01..f9af419 100644
--- a/drivers/net/r6040.c
+++ b/drivers/net/r6040.c
@@ -69,6 +69,8 @@
/* MAC registers */
#define MCR0 0x00 /* Control register 0 */
+#define PROMISC 0x0020 /* Promiscuous mode */
+#define HASH_EN 0x0100 /* Enable multicast hash table function */
#define MCR1 0x04 /* Control register 1 */
#define MAC_RST 0x0001 /* Reset the MAC */
#define MBCR 0x08 /* Bus control */
@@ -935,76 +937,88 @@ static void r6040_multicast_list(struct net_device *dev)
{
struct r6040_private *lp = netdev_priv(dev);
void __iomem *ioaddr = lp->base;
- u16 *adrp;
- u16 reg;
unsigned long flags;
struct dev_mc_list *dmi = dev->mc_list;
int i;
+ u16 hash_table[4] = { 0, };
- /* MAC Address */
- adrp = (u16 *)dev->dev_addr;
- iowrite16(adrp[0], ioaddr + MID_0L);
- iowrite16(adrp[1], ioaddr + MID_0M);
- iowrite16(adrp[2], ioaddr + MID_0H);
-
- /* Promiscous Mode */
spin_lock_irqsave(&lp->lock, flags);
/* Clear AMCP & PROM bits */
- reg = ioread16(ioaddr) & ~0x0120;
+ lp->mcr0 = ioread16(ioaddr + MCR0) & ~(PROMISC | HASH_EN);
+
+ /* Promiscuous Mode */
if (dev->flags & IFF_PROMISC) {
- reg |= 0x0020;
- lp->mcr0 |= 0x0020;
+ lp->mcr0 |= PROMISC;
}
- /* Too many multicast addresses
- * accept all traffic */
- else if ((dev->mc_count > MCAST_MAX)
- || (dev->flags & IFF_ALLMULTI))
- reg |= 0x0020;
+ /* Enable multicast hash table function to
+ * receive all multicast packets. */
+ else if (dev->flags & IFF_ALLMULTI) {
+ lp->mcr0 |= HASH_EN;
+
+ for (i = 0; i < MCAST_MAX ; i++) {
+ iowrite16(0, ioaddr + MID_1L + 8 * i);
+ iowrite16(0, ioaddr + MID_1M + 8 * i);
+ iowrite16(0, ioaddr + MID_1H + 8 * i);
+ }
- iowrite16(reg, ioaddr);
- spin_unlock_irqrestore(&lp->lock, flags);
+ for (i = 0; i < 4; i++)
+ hash_table[i] = 0xffff;
+ }
+ /* Use internal multicast address registers if the number of
+ * multicast addresses is not greater than MCAST_MAX. */
+ else if (dev->mc_count <= MCAST_MAX) {
+ i = 0;
+ while (i < dev->mc_count) {
+ u16 *adrp = (u16 *) dmi->dmi_addr;
+ dmi = dmi->next;
- /* Build the hash table */
- if (dev->mc_count > MCAST_MAX) {
- u16 hash_table[4];
+ iowrite16(adrp[0], ioaddr + MID_1L + 8 * i);
+ iowrite16(adrp[1], ioaddr + MID_1M + 8 * i);
+ iowrite16(adrp[2], ioaddr + MID_1H + 8 * i);
+ i++;
+ }
+ while (i < MCAST_MAX) {
+ iowrite16(0, ioaddr + MID_1L + 8 * i);
+ iowrite16(0, ioaddr + MID_1M + 8 * i);
+ iowrite16(0, ioaddr + MID_1H + 8 * i);
+ i++;
+ }
+ }
+ /* Otherwise, Enable multicast hash table function. */
+ else {
u32 crc;
- for (i = 0; i < 4; i++)
- hash_table[i] = 0;
+ lp->mcr0 |= HASH_EN;
- for (i = 0; i < dev->mc_count; i++) {
- char *addrs = dmi->dmi_addr;
+ for (i = 0; i < MCAST_MAX ; i++) {
+ iowrite16(0, ioaddr + MID_1L + 8 * i);
+ iowrite16(0, ioaddr + MID_1M + 8 * i);
+ iowrite16(0, ioaddr + MID_1H + 8 * i);
+ }
+ /* Build multicast hash table */
+ for (i = 0; i < dev->mc_count; i++) {
+ u8 *addrs = dmi->dmi_addr;
dmi = dmi->next;
- if (!(*addrs & 1))
- continue;
-
- crc = ether_crc_le(6, addrs);
+ crc = ether_crc(ETH_ALEN, addrs);
crc >>= 26;
- hash_table[crc >> 4] |= 1 << (15 - (crc & 0xf));
+ hash_table[crc >> 4] |= 1 << (crc & 0xf);
}
- /* Fill the MAC hash tables with their values */
+
+ }
+ iowrite16(lp->mcr0, ioaddr + MCR0);
+
+ /* Fill the MAC hash tables with their values */
+ if (lp->mcr0 && HASH_EN) {
iowrite16(hash_table[0], ioaddr + MAR0);
iowrite16(hash_table[1], ioaddr + MAR1);
iowrite16(hash_table[2], ioaddr + MAR2);
iowrite16(hash_table[3], ioaddr + MAR3);
}
- /* Multicast Address 1~4 case */
- dmi = dev->mc_list;
- for (i = 0, dmi; (i < dev->mc_count) && (i < MCAST_MAX); i++) {
- adrp = (u16 *)dmi->dmi_addr;
- iowrite16(adrp[0], ioaddr + MID_1L + 8*i);
- iowrite16(adrp[1], ioaddr + MID_1M + 8*i);
- iowrite16(adrp[2], ioaddr + MID_1H + 8*i);
- dmi = dmi->next;
- }
- for (i = dev->mc_count; i < MCAST_MAX; i++) {
- iowrite16(0xffff, ioaddr + MID_1L + 8*i);
- iowrite16(0xffff, ioaddr + MID_1M + 8*i);
- iowrite16(0xffff, ioaddr + MID_1H + 8*i);
- }
+
+ spin_unlock_irqrestore(&lp->lock, flags);
}
static void netdev_get_drvinfo(struct net_device *dev,
---
===========================================================================================
The privileged confidential information contained in this email is intended for use only by the addressees as indicated by the original sender of this email.
If you are not the addressee indicated in this email or are not responsible for delivery of the email to such a person, please kindly reply to the sender indicating this fact and delete all copies of it from your computer and network server immediately.
Your cooperation is highly appreciated. It is advised that any unauthorized use of confidential information of DM&P Group is strictly prohibited; and any information in this email irrelevant to the official business of DM&P Group shall be deemed as neither given nor endorsed by DM&P Group.
===========================================================================================
^ permalink raw reply related
* FINAL NOTIFICATION
From: W.I.N.S @ 2010-11-03 14:20 UTC (permalink / raw)
I am pleased to inform you of the just concluded promotions
that was conducted on the 2nd NOVEMBER in regards to the
EU award promo. in this view you are to collect the
sum of one million Dollars. grant number.
(CT-222-6747,FGN/P-900-56).
Please contact Thomas Herman with the following detail Below
1) Full Names, 2) Full Addresss, 3) Sex, 4) Phone Number,
5) Occupation, 6) Nationality, 7) Age, 8) Country.
Email:
thomasherman@onfruit.cn
Sophia Bufford,
Promo Announcer.
^ permalink raw reply
* [PATCH] xfrm: use gre key as flow upper protocol info
From: Timo Teräs @ 2010-11-03 14:41 UTC (permalink / raw)
To: netdev, Herbert Xu; +Cc: Timo Teräs
The GRE Key field is intended to be used for identifying an individual
traffic flow within a tunnel. It is useful to be able to have XFRM
policy selector matches to have different policies for different
GRE tunnels.
Signed-off-by: Timo Teräs <timo.teras@iki.fi>
---
Basic testing done, but more knowledgeable should check that I did
not miss anything essential.
include/net/flow.h | 2 ++
include/net/xfrm.h | 6 ++++++
net/ipv4/ip_gre.c | 12 +++++++-----
net/ipv4/xfrm4_policy.c | 15 +++++++++++++++
4 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/include/net/flow.h b/include/net/flow.h
index 0ac3fb5..7196e68 100644
--- a/include/net/flow.h
+++ b/include/net/flow.h
@@ -67,6 +67,7 @@ struct flowi {
} dnports;
__be32 spi;
+ __be32 gre_key;
struct {
__u8 type;
@@ -78,6 +79,7 @@ struct flowi {
#define fl_icmp_code uli_u.icmpt.code
#define fl_ipsec_spi uli_u.spi
#define fl_mh_type uli_u.mht.type
+#define fl_gre_key uli_u.gre_key
__u32 secid; /* used by xfrm; see secid.txt */
} __attribute__((__aligned__(BITS_PER_LONG/8)));
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index bcfb6b2..54b2832 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -805,6 +805,9 @@ __be16 xfrm_flowi_sport(struct flowi *fl)
case IPPROTO_MH:
port = htons(fl->fl_mh_type);
break;
+ case IPPROTO_GRE:
+ port = htonl(fl->fl_gre_key) >> 16;
+ break;
default:
port = 0; /*XXX*/
}
@@ -826,6 +829,9 @@ __be16 xfrm_flowi_dport(struct flowi *fl)
case IPPROTO_ICMPV6:
port = htons(fl->fl_icmp_code);
break;
+ case IPPROTO_GRE:
+ port = htonl(fl->fl_gre_key) & 0xffff;
+ break;
default:
port = 0; /*XXX*/
}
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 70ff77f..ffc78e8 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -779,9 +779,9 @@ static netdev_tx_t ipgre_tunnel_xmit(struct sk_buff *skb, struct net_device *dev
.tos = RT_TOS(tos)
}
},
- .proto = IPPROTO_GRE
- }
-;
+ .proto = IPPROTO_GRE,
+ .fl_gre_key = tunnel->parms.o_key
+ };
if (ip_route_output_key(dev_net(dev), &rt, &fl)) {
dev->stats.tx_carrier_errors++;
goto tx_error;
@@ -958,7 +958,8 @@ static int ipgre_tunnel_bind_dev(struct net_device *dev)
.tos = RT_TOS(iph->tos)
}
},
- .proto = IPPROTO_GRE
+ .proto = IPPROTO_GRE,
+ .fl_gre_key = tunnel->parms.o_key
};
struct rtable *rt;
@@ -1223,7 +1224,8 @@ static int ipgre_open(struct net_device *dev)
.tos = RT_TOS(t->parms.iph.tos)
}
},
- .proto = IPPROTO_GRE
+ .proto = IPPROTO_GRE,
+ .fl_gre_key = t->parms.o_key
};
struct rtable *rt;
diff --git a/net/ipv4/xfrm4_policy.c b/net/ipv4/xfrm4_policy.c
index 4464f3b..57af4bd 100644
--- a/net/ipv4/xfrm4_policy.c
+++ b/net/ipv4/xfrm4_policy.c
@@ -11,6 +11,7 @@
#include <linux/err.h>
#include <linux/kernel.h>
#include <linux/inetdevice.h>
+#include <linux/if_tunnel.h>
#include <net/dst.h>
#include <net/xfrm.h>
#include <net/ip.h>
@@ -158,6 +159,20 @@ _decode_session4(struct sk_buff *skb, struct flowi *fl, int reverse)
fl->fl_ipsec_spi = htonl(ntohs(ipcomp_hdr[1]));
}
break;
+
+ case IPPROTO_GRE:
+ if (pskb_may_pull(skb, xprth + 12 - skb->data)) {
+ __be16 *greflags = (__be16 *)xprth;
+ __be32 *gre_hdr = (__be32 *)xprth;
+
+ if (greflags[0] & GRE_KEY) {
+ if (greflags[0] & GRE_CSUM)
+ gre_hdr++;
+ fl->fl_gre_key = gre_hdr[1];
+ }
+ }
+ break;
+
default:
fl->fl_ipsec_spi = 0;
break;
--
1.7.1
^ permalink raw reply related
* Re: [PATCH 1/2] ixgb: Don't check for vlan group on transmit.
From: Jeff Kirsher @ 2010-11-03 14:55 UTC (permalink / raw)
To: Jesse Gross
Cc: David Miller, netdev@vger.kernel.org, Brandeburg, Jesse,
Waskiewicz Jr, Peter P
In-Reply-To: <1288464591-31528-1-git-send-email-jesse@nicira.com>
[-- Attachment #1: Type: text/plain, Size: 830 bytes --]
On Sat, 2010-10-30 at 11:49 -0700, Jesse Gross wrote:
>
> On transmit, the ixgb driver will only use vlan acceleration if a
> vlan group is configured. This can lead to tags getting dropped
> when bridging because the networking core assumes that a driver
> that claims vlan acceleration support can do it at all times. This
> change should have been part of commit eab6d18d "vlan: Don't check for
> vlan group before vlan_tx_tag_present." but was missed.
>
> Signed-off-by: Jesse Gross <jesse@nicira.com>
> CC: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> CC: Jesse Brandeburg <jesse.brandeburg@intel.com>
> CC: PJ Waskiewicz <peter.p.waskiewicz.jr@intel.com>
> ---
> drivers/net/ixgb/ixgb_main.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
Thanks Jesse, I have added this to my queue.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] igb: Don't depend on vlan group for receive size.
From: Jeff Kirsher @ 2010-11-03 14:55 UTC (permalink / raw)
To: Jesse Gross
Cc: David Miller, netdev@vger.kernel.org, Brandeburg, Jesse,
Waskiewicz Jr, Peter P
In-Reply-To: <1288464591-31528-2-git-send-email-jesse@nicira.com>
[-- Attachment #1: Type: text/plain, Size: 676 bytes --]
On Sat, 2010-10-30 at 11:49 -0700, Jesse Gross wrote:
> The igb driver currently sets its maximum receive size based on
> whether a vlan group is configured. However, this causes it to
> completely drop MTU sized vlan packets when there is no vlan
> group, preventing them from showing up in tcpdump, etc.
>
> Signed-off-by: Jesse Gross <jesse@nicira.com>
> CC: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> CC: Jesse Brandeburg <jesse.brandeburg@intel.com>
> CC: PJ Waskiewicz <peter.p.waskiewicz.jr@intel.com>
> ---
> drivers/net/igb/igb_main.c | 5 +----
> 1 files changed, 1 insertions(+), 4 deletions(-)
Thanks, I have added this patch to my queue.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [PATCH net-next-2.6 1/2] be2net: Adding an option to use INTx instead of MSI-X
From: David Miller @ 2010-11-03 15:28 UTC (permalink / raw)
To: michael; +Cc: matthew, bhutchings, somnath.kotur, netdev, linux-pci
In-Reply-To: <1288788357.989.77.camel@concordia>
From: Michael Ellerman <michael@ellerman.id.au>
Date: Wed, 03 Nov 2010 23:45:57 +1100
> On Sat, 2010-10-30 at 17:21 -0600, Matthew Wilcox wrote:
>> On Tue, Oct 26, 2010 at 05:52:08PM +1100, Michael Ellerman wrote:
>> > That horse has really really bolted, it's gawn.
>> >
>> > I count 26 drivers with "disable MSI/X" parameters. Some even have more
>> > than one.
>>
>> That's 26 patches someone needs to write, then. You can put my Acked-by
>> on all of them.
>
> Bah, come on it's hardly the most horrendous sin committed by driver
> writers.
Yes but it's pretty high on the list because it deteriorates the
user experience.
It's a real selfish move to try and "fix" things in this way and
we've established that this isn't acceptable in the past.
^ permalink raw reply
* Unaligned accesses in AVR32 kernel
From: Ralf Baechle @ 2010-11-03 15:32 UTC (permalink / raw)
To: Hans-Christian Egtvedt; +Cc: linux-kernel, netdev, linux-hams, Simon Eatough
Hans-Christian,
I received a bug report for some networking code from which it seems as
if the AVR32 kernel doesn't fixup exceptions caused by missaligned kernel
accesses either in software or hardware. This is a problem as there in
the networking stack there is no general guarantee that all multi-byte
header fields such as ARP or IP headers will aligned. Networking drivers
are supposed to make a decent attempt to place packets at a suitable
address but there is no guarantee. The mkiss.c driver gets that wrong
which I'm fixing but fundamentally you want to add automated fixing of
unaligned exception to the AVR32 kernel. See arch/mips/kernel/unaligned.c.
Cheers,
Ralf
^ permalink raw reply
* [PATCH] mpc52xx: cleanup locking
From: Eric Dumazet @ 2010-11-03 15:56 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Asier Llano, Grant Likely, Jean-Michel Hautbois
commit 1e4e0767ecb1 (Fix locking on fec_mpc52xx driver) assumed IRQ are
enabled when an IRQ handler is called.
It is not the case anymore (IRQF_DISABLED is deprecated), so we can use
regular spin_lock(), no need for spin_lock_irqsave().
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Tested-by: Jean-Michel Hautbois <jhautbois@gmail.com>
Cc: Asier Llano <a.llano@ziv.es>
Cc: Grant Likely <grant.likely@secretlab.ca>
---
drivers/net/fec_mpc52xx.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/drivers/net/fec_mpc52xx.c b/drivers/net/fec_mpc52xx.c
index e9f5d03..50c1213 100644
--- a/drivers/net/fec_mpc52xx.c
+++ b/drivers/net/fec_mpc52xx.c
@@ -366,9 +366,8 @@ static irqreturn_t mpc52xx_fec_tx_interrupt(int irq, void *dev_id)
{
struct net_device *dev = dev_id;
struct mpc52xx_fec_priv *priv = netdev_priv(dev);
- unsigned long flags;
- spin_lock_irqsave(&priv->lock, flags);
+ spin_lock(&priv->lock);
while (bcom_buffer_done(priv->tx_dmatsk)) {
struct sk_buff *skb;
struct bcom_fec_bd *bd;
@@ -379,7 +378,7 @@ static irqreturn_t mpc52xx_fec_tx_interrupt(int irq, void *dev_id)
dev_kfree_skb_irq(skb);
}
- spin_unlock_irqrestore(&priv->lock, flags);
+ spin_unlock(&priv->lock);
netif_wake_queue(dev);
@@ -395,9 +394,8 @@ static irqreturn_t mpc52xx_fec_rx_interrupt(int irq, void *dev_id)
struct bcom_fec_bd *bd;
u32 status, physaddr;
int length;
- unsigned long flags;
- spin_lock_irqsave(&priv->lock, flags);
+ spin_lock(&priv->lock);
while (bcom_buffer_done(priv->rx_dmatsk)) {
@@ -429,7 +427,7 @@ static irqreturn_t mpc52xx_fec_rx_interrupt(int irq, void *dev_id)
/* Process the received skb - Drop the spin lock while
* calling into the network stack */
- spin_unlock_irqrestore(&priv->lock, flags);
+ spin_unlock(&priv->lock);
dma_unmap_single(dev->dev.parent, physaddr, rskb->len,
DMA_FROM_DEVICE);
@@ -438,10 +436,10 @@ static irqreturn_t mpc52xx_fec_rx_interrupt(int irq, void *dev_id)
rskb->protocol = eth_type_trans(rskb, dev);
netif_rx(rskb);
- spin_lock_irqsave(&priv->lock, flags);
+ spin_lock(&priv->lock);
}
- spin_unlock_irqrestore(&priv->lock, flags);
+ spin_unlock(&priv->lock);
return IRQ_HANDLED;
}
@@ -452,7 +450,6 @@ static irqreturn_t mpc52xx_fec_interrupt(int irq, void *dev_id)
struct mpc52xx_fec_priv *priv = netdev_priv(dev);
struct mpc52xx_fec __iomem *fec = priv->fec;
u32 ievent;
- unsigned long flags;
ievent = in_be32(&fec->ievent);
@@ -470,9 +467,9 @@ static irqreturn_t mpc52xx_fec_interrupt(int irq, void *dev_id)
if (net_ratelimit() && (ievent & FEC_IEVENT_XFIFO_ERROR))
dev_warn(&dev->dev, "FEC_IEVENT_XFIFO_ERROR\n");
- spin_lock_irqsave(&priv->lock, flags);
+ spin_lock(&priv->lock);
mpc52xx_fec_reset(dev);
- spin_unlock_irqrestore(&priv->lock, flags);
+ spin_unlock(&priv->lock);
return IRQ_HANDLED;
}
^ permalink raw reply related
* Re: [PATCH net-next-2.6 v2] can: Topcliff: PCH_CAN driver: Fix build warnings
From: Wolfgang Grandegger @ 2010-11-03 16:15 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w, Samuel Ortiz,
margie.foster-ral2JQCrhuEAvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
yong.y.wang-ral2JQCrhuEAvxtiuMwx3w, Masayuki Ohtake,
kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w, Christian Pellegrin,
David S. Miller, joel.clark-ral2JQCrhuEAvxtiuMwx3w,
qi.wang-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <4CCE9F0B.1000901-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
On 11/01/2010 12:05 PM, Marc Kleine-Budde wrote:
> Hello,
>
> On 10/29/2010 09:32 PM, Wolfgang Grandegger wrote:
>
> some comments inline.
>
> [...]
...
>>>> + if (status & PCH_LEC_ALL) {
>>>> + priv->can.can_stats.bus_error++;
>>>> + stats->rx_errors++;
>>>> + switch (status & PCH_LEC_ALL) {
>>>
>>> I suggest to convert to a if-bit-set because there might be more than
>>> one bit set.
>>
>> Marc, what do you mean here. It's an enumeraton. Maybe the following
>> code is more clear:
>
> Oh, I haven't looked this one up in the datasheet.
>>
>> lec = status & PCH_LEC_ALL;
>> if (lec > 0) {
>
> or "if (lec)", YMMV
>
>> switch (lec) {
>>
>>>> + case PCH_STUF_ERR:
>>>> + cf->data[2] |= CAN_ERR_PROT_STUFF;
>>>> + break;
>>>> + case PCH_FORM_ERR:
>>>> + cf->data[2] |= CAN_ERR_PROT_FORM;
>>>> + break;
>>>> + case PCH_ACK_ERR:
>>>> + cf->data[2] |= CAN_ERR_PROT_LOC_ACK |
>>>> + CAN_ERR_PROT_LOC_ACK_DEL;
>>
>> Could you check what that type of bus error that is? Usually it's a ack
>> lost error.
>>
>>>> + break;
>>>> + case PCH_BIT1_ERR:
>>>> + case PCH_BIT0_ERR:
>>>> + cf->data[2] |= CAN_ERR_PROT_BIT;
>>>> + break;
>>>> + case PCH_CRC_ERR:
>>>> + cf->data[2] |= CAN_ERR_PROT_LOC_CRC_SEQ |
>>>> + CAN_ERR_PROT_LOC_CRC_DEL;
>>>> + break;
>>>> + default:
>>>> + iowrite32(status | PCH_LEC_ALL, &priv->regs->stat);
>>>> + break;
>>>> + }
>>>> +
>>>> + }
At a closer look, I doubt that the above LEC handling is correct.
According to the manual: "when the LEC shows the value “7,” this value
was written to the LEC by the CPU; thus, no CAN bus event was detected"
Therefore the LEC bits should be properly *initialized* and *reset* to
0x7, which I do not find in the code.
Wolfgang.
^ permalink raw reply
* [PATCH 0/1] UDEV - Rename onboard network interfaces to lomN if the user so desires
From: Narendra_K @ 2010-11-03 16:49 UTC (permalink / raw)
To: linux-hotplug; +Cc: netdev, Matt_Domsch, Jordan_Hargrave, Charles_Rose
Hello,
This patch adds support to udev's dynamic rule generation mechanism to
rename onboard network interfaces to lom1, lom2 etc if the user so
desires. (Please refer to this for more information -
http://marc.info/?l=linux-netdev&m=128646170613973&w=3).
From: Narendra K <narendra_k@dell.com>
Subject: [PATCH] UDEV - Rename onboard network interfaces to lomN if the user so desires
This patch adds support to udev's dynamic rule generation mechanism to
rename onboard network interfaces to lom1, lom2 etc if the user so
desires. (Please refer to this for more information -
http://marc.info/?l=linux-netdev&m=128646170613973&w=3).
It introduces a commad line parameter 'udevlom', which when passed
would rename onboard network interfaces to lomN. It would also generate
corresponding rules to make the names persistent across reboots.
With this patch, interface with firmware index=1 will be renamed to lom1,
index=2 will be rename to lom2 etc.(lom - Lan-On-Motherboard)
This patch is against Fedora 14 udev (version:161 release:4.fc14).
It requires the upstream commit 911e1c9b05a8e3559a7aa89083930700a0b9e7ee
for the firmware index to be available in sysfs.
With the patch applied, the network interfaces look like this on a
Dell PowerEdge R710 with four BCM5709 onboard NICs with firmware
indexes and four add-in interfaces.
[root@fedora-14-r710 ~]# ls /sys/class/net/
eth4 eth5 eth6 eth7 eth8 lo lom1 lom2 lom3 lom4
/etc/udev/rules.d/70-persistent-net.rules would look like this -
[snippet attched]
SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*", ATTR{address}=="00:24:e8:2e:df:01", ATTR{dev_id}=="0x0", ATTR{type}=="1", ATTRS{index}=="2", KERNEL=="eth*", NAME="lom2"
SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*", ATTR{address}=="00:15:17:15:9b:eb", ATTR{dev_id}=="0x0", ATTR{type}=="1", KERNEL=="eth*", NAME="eth4"
SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*", ATTR{address}=="00:24:e8:2e:df:05", ATTR{dev_id}=="0x0", ATTR{type}=="1", ATTRS{index}=="4", KERNEL=="eth*", NAME="lom4"
SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*", ATTR{address}=="00:24:e8:2e:df:03", ATTR{dev_id}=="0x0", ATTR{type}=="1", ATTRS{index}=="3", KERNEL=="eth*", NAME="lom3"
SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*", ATTR{address}=="00:24:e8:2e:de:ff", ATTR{dev_id}=="0x0", ATTR{type}=="1", ATTRS{index}=="1", KERNEL=="eth*", NAME="lom1"
Signed-off-by: Narendra K <narendra_k@dell.com>
---
.../75-persistent-net-generator.rules | 4 ++++
| 15 ++++++++++++++-
2 files changed, 18 insertions(+), 1 deletions(-)
diff --git a/extras/rule_generator/75-persistent-net-generator.rules b/extras/rule_generator/75-persistent-net-generator.rules
index 8119d0e..e138bd3 100644
--- a/extras/rule_generator/75-persistent-net-generator.rules
+++ b/extras/rule_generator/75-persistent-net-generator.rules
@@ -7,6 +7,7 @@
# MATCHID bus_id used for the match
# MATCHDRV driver name used for the match
# MATCHIFTYPE interface type match
+# MATCHINDEX firmware index used for the match
# COMMENT comment to add to the generated rule
# INTERFACE_NAME requested name supplied by external tool
# INTERFACE_NEW new interface name returned by rule writer
@@ -29,6 +30,9 @@ ENV{MATCHADDR}="$attr{address}"
# match interface type
ENV{MATCHIFTYPE}="$attr{type}"
+#read firmware index
+ATTRS{index}=="?*", ENV{MATCHINDEX}="$attr{index}"
+
# ignore KVM virtual interfaces
ENV{MATCHADDR}=="54:52:00:*", GOTO="persistent_net_generator_end"
--git a/extras/rule_generator/write_net_rules b/extras/rule_generator/write_net_rules
index 4379792..40aaa4b 100644
--- a/extras/rule_generator/write_net_rules
+++ b/extras/rule_generator/write_net_rules
@@ -11,6 +11,7 @@
# MATCHDEVID dev_id used for the match
# MATCHDRV driver name used for the match
# MATCHIFTYPE interface type match
+# MATCHINDEX firmware index used for the match
# COMMENT comment to add to the generated rule
# INTERFACE_NAME requested name supplied by external tool
# INTERFACE_NEW new interface name returned by rule writer
@@ -72,7 +73,11 @@ write_rule() {
echo ""
[ "$comment" ] && echo "# $comment"
- echo "SUBSYSTEM==\"net\", ACTION==\"add\"$match, NAME=\"$name\""
+ if [ "$MATCHINDEX" -a "$UDEVLOM" ]; then
+ echo "SUBSYSTEM==\"net\", ACTION==\"add\"$match, NAME=\"lom$MATCHINDEX\""
+ else
+ echo "SUBSYSTEM==\"net\", ACTION==\"add\"$match, NAME=\"$name\""
+ fi
} >> $RULES_FILE
}
@@ -108,6 +113,10 @@ if [ "$MATCHIFTYPE" ]; then
match="$match, ATTR{type}==\"$MATCHIFTYPE\""
fi
+if [ "$MATCHINDEX" -a "$UDEVLOM" ]; then
+ match="$match, ATTRS{index}==\"$MATCHINDEX\""
+fi
+
if [ -z "$match" ]; then
echo "missing valid match" >&2
unlock_rules_file
@@ -134,6 +143,10 @@ else
fi
fi
+if [ "$MATCHINDEX" -a "$UDEVLOM" ]; then
+ echo "INTERFACE_NEW=lom$MATCHINDEX"
+fi
+
write_rule "$match" "$INTERFACE" "$COMMENT"
unlock_rules_file
--
1.7.3.1
With regards,
Narendra K
^ permalink raw reply related
* [PATCH 1/1] UDEV - Add 'udevlom' command line param to start_udev
From: Narendra_K @ 2010-11-03 16:55 UTC (permalink / raw)
To: linux-hotplug; +Cc: netdev, Matt_Domsch, Jordan_Hargrave, Charles_Rose
Hello,
This patch allows users to specify if they want the onboard network
interfaces to be renamed to lomN by implementing a command line param
'udevlom'.
From: Narendra K <narendra_k@dell.com>
Subject: [PATCH] UDEV - Add 'udevlom' command line param to start_udev
This patch implements 'udevlom' command line parameter, which
when passed, results in onboard network interfaces getting
renamed to lomN.
Signed-off-by: Narendra K <narendra_k@dell.com>
---
start_udev | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/start_udev b/start_udev
index 49fc286..57d60c9 100755
--- a/start_udev
+++ b/start_udev
@@ -32,6 +32,7 @@ export TZ=/etc/localtime
. /etc/init.d/functions
prog=udev
+cmdline=`cat /proc/cmdline`
touch_recursive() {
( cd $1;
@@ -60,6 +61,10 @@ fi
ret=$[$ret + $?]
+if strstr "$cmdline" udevlom; then
+ /sbin/udevadm control --env=UDEVLOM="y"
+fi
+
/sbin/udevadm trigger --type=subsystems --action=add
/sbin/udevadm trigger --type=devices --action=add
/sbin/udevadm settle
--
1.7.3.1
With regards,
Narendra K
^ permalink raw reply related
* Re: [PATCH] drivers/net/tile/: on-chip network drivers for the tile architecture
From: Stephen Hemminger @ 2010-11-03 16:59 UTC (permalink / raw)
To: Chris Metcalf; +Cc: linux-kernel, netdev
In-Reply-To: <201011012107.oA1L7TGd031588@farm-0027.internal.tilera.com>
On Mon, 1 Nov 2010 17:00:37 -0400
Chris Metcalf <cmetcalf@tilera.com> wrote:
> This change adds the first network driver for the tile architecture,
> supporting the on-chip XGBE and GBE shims.
>
> The infrastructure is present for the TILE-Gx networking drivers (another
> three source files in the new directory) but for now the the actual
> tilegx sources are waiting on releasing hardware to initial customers.
>
> Note that arch/tile/include/hv/* are "upstream" headers from the
> Tilera hypervisor and will probably benefit less from LKML review.
>
> Signed-off-by: Chris Metcalf <cmetcalf@tilera.com>
> +typedef struct {
> + /** Byte offset of the next notify packet to be written: zero for the first
> + * packet on the queue, sizeof (netio_pkt_t) for the second packet on the
> + * queue, etc. */
> + volatile uint32_t __packet_write;
> +
> + /** Offset of the packet after the last valid packet (i.e., when any
> + * pointer is incremented to this value, it wraps back to zero). */
> + uint32_t __last_packet_plus_one;
> +}
> +__netio_packet_queue_t;
1. MUST not use volatile, see volatile-considered-harmful.txt
2. SHOULD use __u32 rather than uint32_t in kernel structures
3. MUST not introduce typedef's; use structures
4. SHOULD use proper kernel implementation
If you use scripts/checkpatch.pl it will tell you about these.
^ permalink raw reply
* Re: [PATCH] drivers/net/tile/: on-chip network drivers for the tile architecture
From: Chris Metcalf @ 2010-11-03 17:37 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: linux-kernel, netdev
In-Reply-To: <20101103125959.3231daa1@s6510>
Stephen, thanks for your feedback!
On 11/3/2010 12:59 PM, Stephen Hemminger wrote:
> 1. MUST not use volatile, see volatile-considered-harmful.txt
The "harmful" use of volatile is in trying to fake out SMP. Believe me,
with a 64-core architecture, we know our SMP guidelines. :-) Our use here
is simply to force the compiler to issue a load, for the side-effect of
populating the TLB, for example.
However, your response does suggest that simply the syntactic use of
"volatile" will cause a red flag for readers. I'll move this to an inline
function in a header with a comment explaining what it's for, and use the
function instead.
> 2. SHOULD use __u32 rather than uint32_t in kernel structures
Thanks, I've made this change in drivers/net/tile/tilepro.c. In fact, I
used "u32" since this code is not shared with userspace. However, see
below for the <hv/xxx.h> header files.
> 3. MUST not introduce typedef's; use structures
> 4. SHOULD use proper kernel implementation
(I think you mean proper kernel indentation.) This is already true for the
kernel sources in driver/net/tile/, but false for the tile-specific
hypervisor headers in arch/tile/include/hv/. These are "upstream" headers
that are being added to the kernel on an as-needed basis so the kernel can
talk with the hypervisor.
Including a copy of the hypervisor headers with the kernel makes it easier
to build the kernel (since there are no external dependencies that need to
be tracked down to do the build) and is consistent with the fact that we
can in principle modify the hypervisor and its headers out of sync with the
kernel, but still expect the old headers to work correctly with a new
hypervisor since Linux always initializes itself to the hypervisor with the
compiled-in header version number.
So the upshot is that these headers are imported into the kernel and
generally can't be stylistically modified. But they are "ghettoized" to
arch/tile/include/hv/ and should not cause any trouble. :-)
> If you use scripts/checkpatch.pl it will tell you about these.
Yes, the "volatile" issue is mentioned, but even with "--strict", there's
no mention of uint32_t, so I missed that. I don't see "uint32" mentioned
anywhere in scripts/checkpatch.pl, in fact.
--
Chris Metcalf, Tilera Corp.
http://www.tilera.com
^ permalink raw reply
* Re: [PATCH] drivers/net/tile/: on-chip network drivers for the tile architecture
From: Eric Dumazet @ 2010-11-03 17:50 UTC (permalink / raw)
To: Chris Metcalf; +Cc: Stephen Hemminger, linux-kernel, netdev
In-Reply-To: <4CD19DCF.1040709@tilera.com>
Le mercredi 03 novembre 2010 à 13:37 -0400, Chris Metcalf a écrit :
> Stephen, thanks for your feedback!
>
> On 11/3/2010 12:59 PM, Stephen Hemminger wrote:
> > 1. MUST not use volatile, see volatile-considered-harmful.txt
>
> The "harmful" use of volatile is in trying to fake out SMP. Believe me,
> with a 64-core architecture, we know our SMP guidelines. :-) Our use here
> is simply to force the compiler to issue a load, for the side-effect of
> populating the TLB, for example.
>
> However, your response does suggest that simply the syntactic use of
> "volatile" will cause a red flag for readers. I'll move this to an inline
> function in a header with a comment explaining what it's for, and use the
> function instead.
Please read Documentation/volatile-considered-harmful.txt
Then if there is a problem, we can make change to the documentation, but
volatile use in new code is _strictly_ forbidden.
ACCESS_ONCE() is your friend, we might document it in
Documentation/volatile-considered-harmful.txt
^ permalink raw reply
* Re: [PATCH 1/1] UDEV - Add 'udevlom' command line param to start_udev
From: Greg KH @ 2010-11-03 18:05 UTC (permalink / raw)
To: Narendra_K
Cc: linux-hotplug, netdev, Matt_Domsch, Jordan_Hargrave, Charles_Rose
In-Reply-To: <20101103165505.GA3281@fedora-14-r710.oslab.blr.amer.dell.com>
On Wed, Nov 03, 2010 at 10:25:25PM +0530, Narendra_K@Dell.com wrote:
> Hello,
>
> This patch allows users to specify if they want the onboard network
> interfaces to be renamed to lomN by implementing a command line param
> 'udevlom'.
Ick ick ick.
Why not do this in some other configuration file? Don't rely on udev
being started with a different option, that is only ripe for abuse by
everyone else who wants their pet-project to get into the udev
environment.
Please, surely there's a different way to do this.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 1/1] UDEV - Add 'udevlom' command line param to start_udev
From: Marco d'Itri @ 2010-11-03 18:32 UTC (permalink / raw)
To: Greg KH
Cc: Narendra_K, linux-hotplug, netdev, Matt_Domsch, Jordan_Hargrave,
Charles_Rose
In-Reply-To: <20101103180500.GA7441@kroah.com>
[-- Attachment #1: Type: text/plain, Size: 505 bytes --]
On Nov 03, Greg KH <greg@kroah.com> wrote:
> Why not do this in some other configuration file? Don't rely on udev
> being started with a different option, that is only ripe for abuse by
> everyone else who wants their pet-project to get into the udev
> environment.
Agreed. What about instructing users/installers to add something like
this in /etc/udev/rules.d/00-something.rules?
KERNEL=="eth*", ENV{UDEVLOM}="1"
(Maybe with a more descriptive name than "UDEVLOM".)
--
ciao,
Marco
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [PATCH] bluetooth: bnep: fix information leak to userland
From: Gustavo F. Padovan @ 2010-11-03 18:36 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Vasiliy Kulikov, kernel-janitors, David S. Miller, Eric Dumazet,
Thadeu Lima de Souza Cascardo, Tejun Heo, Jiri Kosina,
linux-bluetooth, netdev, linux-kernel
In-Reply-To: <1288712131.3322.178.camel@aeonflux>
* Marcel Holtmann <marcel@holtmann.org> [2010-11-02 16:35:31 +0100]:
> Hi Vasiiy,
>
> > Structure bnep_conninfo is copied to userland with the field "device"
> > that has the last elements unitialized. It leads to leaking of
> > contents of kernel stack memory.
> >
> > Signed-off-by: Vasiliy Kulikov <segooon@gmail.com>
>
> Acked-by: Marcel Holtmann <marcel@holtmann.org>
Applied, thanks.
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* Re: [PATCH] bluetooth: cmtp: fix information leak to userland
From: Gustavo F. Padovan @ 2010-11-03 18:36 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Vasiliy Kulikov, kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
David S. Miller, Eric Dumazet,
linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1288712158.3322.179.camel@aeonflux>
* Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org> [2010-11-02 16:35:58 +0100]:
> Hi Vasiliy,
>
> > Structure cmtp_conninfo is copied to userland with some padding fields
> > unitialized. It leads to leaking of contents of kernel stack memory.
> >
> > Signed-off-by: Vasiliy Kulikov <segooon-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> Acked-by: Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>
Applied, thanks.
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* Re: [PATCH] bluetooth: hidp: fix information leak to userland
From: Gustavo F. Padovan @ 2010-11-03 18:37 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Vasiliy Kulikov, kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
David S. Miller, Jiri Kosina, Michael Poole, Bastien Nocera,
linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1288712189.3322.180.camel@aeonflux>
* Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org> [2010-11-02 16:36:29 +0100]:
> Hi Vasiliy,
>
> > Structure hidp_conninfo is copied to userland with version, product,
> > vendor and name fields unitialized if both session->input and session->hid
> > are NULL. It leads to leaking of contents of kernel stack memory.
> >
> > Signed-off-by: Vasiliy Kulikov <segooon-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> Acked-by: Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>
Applied, thanks.
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* Re: [PATCH] drivers/net/tile/: on-chip network drivers for the tile architecture
From: Chris Metcalf @ 2010-11-03 19:39 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Stephen Hemminger, linux-kernel, netdev
In-Reply-To: <1288806622.2511.187.camel@edumazet-laptop>
On 11/3/2010 1:50 PM, Eric Dumazet wrote:
> Le mercredi 03 novembre 2010 à 13:37 -0400, Chris Metcalf a écrit :
>> Stephen, thanks for your feedback!
>>
>> On 11/3/2010 12:59 PM, Stephen Hemminger wrote:
>>> 1. MUST not use volatile, see volatile-considered-harmful.txt
>> The "harmful" use of volatile is in trying to fake out SMP. Believe me,
>> with a 64-core architecture, we know our SMP guidelines. :-) Our use here
>> is simply to force the compiler to issue a load, for the side-effect of
>> populating the TLB, for example.
>>
>> However, your response does suggest that simply the syntactic use of
>> "volatile" will cause a red flag for readers. I'll move this to an inline
>> function in a header with a comment explaining what it's for, and use the
>> function instead.
> Please read Documentation/volatile-considered-harmful.txt
I read it and internalized it long ago, and re-read it when I got Stephen's
original email. I should have said that explicitly instead of a comment
with a smiley -- email is a tricky communication medium sometimes.
Several uses of "*(volatile int *)ptr" in that file are intended as
performance hints. A more obvious way to state this, for our compiler, is
to say "prefetch_L1(ptr)". This generates essentially the same code, but
avoids the red flag for "volatile" and also reads more clearly, so it's a
good change.
The other use is part of a very precise dance that involves detailed
knowledge of the Tile memory subsystem micro-architecture. This doesn't
really belong in the network device driver code, so I've moved it to
<asm/cacheflush.h>, and cleaned it up, with detailed comments. The use
here is that our network hardware's DMA engine can be used in a mode where
it reads directly from memory, in which case you must ensure that any
cached values have been flushed.
/*
* Flush & invalidate a VA range that is homed remotely on a single core,
* waiting until the memory controller holds the flushed values.
*/
static inline void finv_buffer_remote(void *buffer, size_t size)
{
char *p;
int i;
/*
* Flush and invalidate the buffer out of the local L1/L2
* and request the home cache to flush and invalidate as well.
*/
__finv_buffer(buffer, size);
/*
* Wait for the home cache to acknowledge that it has processed
* all the flush-and-invalidate requests. This does not mean
* that the flushed data has reached the memory controller yet,
* but it does mean the home cache is processing the flushes.
*/
__insn_mf();
/*
* Issue a load to the last cache line, which can't complete
* until all the previously-issued flushes to the same memory
* controller have also completed. If we weren't striping
* memory, that one load would be sufficient, but since we may
* be, we also need to back up to the last load issued to
* another memory controller, which would be the point where
* we crossed an 8KB boundary (the granularity of striping
* across memory controllers). Keep backing up and doing this
* until we are before the beginning of the buffer, or have
* hit all the controllers.
*/
for (i = 0, p = (char *)buffer + size - 1;
i < (1 << CHIP_LOG_NUM_MSHIMS()) && p >= (char *)buffer;
++i) {
const unsigned long STRIPE_WIDTH = 8192;
/* Force a load instruction to issue. */
*(volatile char *)p;
/* Jump to end of previous stripe. */
p -= STRIPE_WIDTH;
p = (char *)((unsigned long)p | (STRIPE_WIDTH - 1));
}
/* Wait for the loads (and thus flushes) to have completed. */
__insn_mf();
}
> Then if there is a problem, we can make change to the documentation, but
> volatile use in new code is _strictly_ forbidden.
>
> ACCESS_ONCE() is your friend, we might document it in
> Documentation/volatile-considered-harmful.txt
Good idea, but neither of the use cases at issue here benefit from ACCESS_ONCE.
Thanks for your feedback!
--
Chris Metcalf, Tilera Corp.
http://www.tilera.com
^ permalink raw reply
* Re: [PATCH] xfrm: use gre key as flow upper protocol info
From: Michał Mirosław @ 2010-11-03 20:16 UTC (permalink / raw)
To: Timo Teräs; +Cc: netdev, Herbert Xu
In-Reply-To: <1288795298-323-1-git-send-email-timo.teras@iki.fi>
2010/11/3 Timo Teräs <timo.teras@iki.fi>:
> The GRE Key field is intended to be used for identifying an individual
> traffic flow within a tunnel. It is useful to be able to have XFRM
> policy selector matches to have different policies for different
> GRE tunnels.
[...]
> diff --git a/net/ipv4/xfrm4_policy.c b/net/ipv4/xfrm4_policy.c
> index 4464f3b..57af4bd 100644
> --- a/net/ipv4/xfrm4_policy.c
> +++ b/net/ipv4/xfrm4_policy.c
> @@ -158,6 +159,20 @@ _decode_session4(struct sk_buff *skb, struct flowi *fl, int reverse)
> fl->fl_ipsec_spi = htonl(ntohs(ipcomp_hdr[1]));
> }
> break;
> +
> + case IPPROTO_GRE:
> + if (pskb_may_pull(skb, xprth + 12 - skb->data)) {
> + __be16 *greflags = (__be16 *)xprth;
> + __be32 *gre_hdr = (__be32 *)xprth;
> +
> + if (greflags[0] & GRE_KEY) {
> + if (greflags[0] & GRE_CSUM)
> + gre_hdr++;
> + fl->fl_gre_key = gre_hdr[1];
> + }
> + }
> + break;
> +
> default:
> fl->fl_ipsec_spi = 0;
> break;
I would expect that keyless tunnel would be separate from key 0 tunnel.
Best Regards,
Michał Mirosław
^ permalink raw reply
* [PATCH] caif: Remove noisy printout when disconnecting caif socket
From: Sjur Braendeland @ 2010-11-03 20:19 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Sjur Braendeland
Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---
net/caif/cfctrl.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/net/caif/cfctrl.c b/net/caif/cfctrl.c
index 08f267a..3cd8f97 100644
--- a/net/caif/cfctrl.c
+++ b/net/caif/cfctrl.c
@@ -361,11 +361,10 @@ void cfctrl_cancel_req(struct cflayer *layr, struct cflayer *adap_layer)
struct cfctrl_request_info *p, *tmp;
struct cfctrl *ctrl = container_obj(layr);
spin_lock(&ctrl->info_list_lock);
- pr_warn("enter\n");
list_for_each_entry_safe(p, tmp, &ctrl->list, list) {
if (p->client_layer == adap_layer) {
- pr_warn("cancel req :%d\n", p->sequence_no);
+ pr_debug("cancel req :%d\n", p->sequence_no);
list_del(&p->list);
kfree(p);
}
--
1.6.3.3
^ permalink raw reply related
* Re: [PATCH] drivers/net/tile/: on-chip network drivers for the tile architecture
From: Eric Dumazet @ 2010-11-03 20:31 UTC (permalink / raw)
To: Chris Metcalf; +Cc: Stephen Hemminger, linux-kernel, netdev
In-Reply-To: <4CD1BA71.3000806@tilera.com>
Le mercredi 03 novembre 2010 à 15:39 -0400, Chris Metcalf a écrit :
> I read it and internalized it long ago, and re-read it when I got Stephen's
> original email. I should have said that explicitly instead of a comment
> with a smiley -- email is a tricky communication medium sometimes.
>
> Several uses of "*(volatile int *)ptr" in that file are intended as
> performance hints. A more obvious way to state this, for our compiler, is
> to say "prefetch_L1(ptr)". This generates essentially the same code, but
> avoids the red flag for "volatile" and also reads more clearly, so it's a
> good change.
>
> The other use is part of a very precise dance that involves detailed
> knowledge of the Tile memory subsystem micro-architecture. This doesn't
> really belong in the network device driver code, so I've moved it to
> <asm/cacheflush.h>, and cleaned it up, with detailed comments. The use
> here is that our network hardware's DMA engine can be used in a mode where
> it reads directly from memory, in which case you must ensure that any
> cached values have been flushed.
>
This kind of things really must be discussed before using it in a
network driver.
Because, an skb can be built by one CPU, queued on a qdisc queue, with
no particular "cached values have been flushed" ...
It then can be dequeued by another CPU, and given to the device.
What happens then ?
> /*
> * Flush & invalidate a VA range that is homed remotely on a single core,
> * waiting until the memory controller holds the flushed values.
> */
> static inline void finv_buffer_remote(void *buffer, size_t size)
> {
> char *p;
> int i;
>
> /*
> * Flush and invalidate the buffer out of the local L1/L2
> * and request the home cache to flush and invalidate as well.
> */
> __finv_buffer(buffer, size);
>
> /*
> * Wait for the home cache to acknowledge that it has processed
> * all the flush-and-invalidate requests. This does not mean
> * that the flushed data has reached the memory controller yet,
> * but it does mean the home cache is processing the flushes.
> */
> __insn_mf();
>
> /*
> * Issue a load to the last cache line, which can't complete
> * until all the previously-issued flushes to the same memory
> * controller have also completed. If we weren't striping
> * memory, that one load would be sufficient, but since we may
> * be, we also need to back up to the last load issued to
> * another memory controller, which would be the point where
> * we crossed an 8KB boundary (the granularity of striping
> * across memory controllers). Keep backing up and doing this
> * until we are before the beginning of the buffer, or have
> * hit all the controllers.
> */
> for (i = 0, p = (char *)buffer + size - 1;
> i < (1 << CHIP_LOG_NUM_MSHIMS()) && p >= (char *)buffer;
> ++i) {
> const unsigned long STRIPE_WIDTH = 8192;
>
> /* Force a load instruction to issue. */
> *(volatile char *)p;
>
> /* Jump to end of previous stripe. */
> p -= STRIPE_WIDTH;
> p = (char *)((unsigned long)p | (STRIPE_WIDTH - 1));
> }
>
> /* Wait for the loads (and thus flushes) to have completed. */
> __insn_mf();
> }
>
^ permalink raw reply
* Netfilter MARK on tc ingress and ifb redirect
From: Luciano Ruete @ 2010-11-03 20:25 UTC (permalink / raw)
To: netdev
Hi,
Im developing a FLOSS ISP solution based on iptables/tc/iproute2
2 stumbling blocks that I found in my path
1) It would be very usefull have this working
tc filter add dev eth0 parent :ffff ... action ipt -j CONNMARK --restore-mark
where :ffff is an ingress qdisc, i know that currently this is not working nor
coded.
Is this anyhow in the sight or TODO list of the iproute2 developers to have
connmark available in ingress?
If not, how complex will be to implement it? (ie: lines of code number)
2) For a technical reason we need to be able to do:
tc filter action mirred egress redirect dev ifbx
at least twice in the same qdisc tree or nested in the redirected ifb, now
only the first filter matched returns. That was possible in (i think) pre 2.6.18
kernels but changed to avoid an infinite loop.
Is there any chance to have that behavior back using a kernel flag or
something?
PD: Plz CC me i'm not suscribed (I try but never get the reply)
--
Luciano Ruete
Sequre - Sys Admin
Mitre 617, piso 7, of. 1
+54 261 4254894
Mendoza - Argentina
http://www.sequre.com.ar/
^ 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