* Re: [PATCH] ipv4: devconf: start IPV4_DEVCONF_* from 0
From: Lucian Adrian Grijincu @ 2011-01-13 7:50 UTC (permalink / raw)
To: David Miller
Cc: netdev, tgraf, kuznet, pekkas, jmorris, yoshfuji, kaber, opurdila,
ddvlad
In-Reply-To: <20110112.184734.38051229.davem@davemloft.net>
On Thu, Jan 13, 2011 at 4:47 AM, David Miller <davem@davemloft.net> wrote:
> From: Lucian Adrian Grijincu <lucian.grijincu@gmail.com>
> Date: Wed, 12 Jan 2011 12:19:10 +0200
>
>> The IPV4_DEVCONF_* enums are never exposed to the userspace and it
>> would make code simpler to remove all the useless (-1) adjustments.
>
> Starting values like this at "1" is usually done on purpose.
>
> It allows "0" to be illegal or mean "none", and thus easily trapping
> cases where the value fails to be initialized properly. In this way
> the illegal sentinel "0" doesn't take up any space either.
Just that no one checks for zero as invalid anywhere.
We pass the enum names everywhere as parameters. And wherever we need
to use those values we must make sure to subtract 1 every time.
And some things work ok, but it's not entirely obvious why.
For example:
struct ctl_table devinet_vars[__IPV4_DEVCONF_MAX] (...) = {
DEVINET_SYSCTL_COMPLEX_ENTRY(FORWARDING, "forwarding",
devinet_sysctl_forward),
This works ok because the DEVINET_SYSCTL_* macros subtract 1 from each
array index.
Because the size of the array is __IPV4_DEVCONF_MAX (without
subtracting 1), there's an extra element at the end and because this
is a global definition it gets initialized with zeros just as required
by register_net_sysctl_table: the last element's procname must be zero
to indicate end-of-array.
Yes it works, but there does not seem to be a good reason why to
complicate things like this (again the sentinel nature of zero is not
used in any place here).
--
.
..: Lucian
^ permalink raw reply
* Re: [PATCH net-next-2.6] netdev: tilepro: Use is_multicast_ether_addr helper
From: Tobias Klauser @ 2011-01-13 8:13 UTC (permalink / raw)
To: David Miller; +Cc: cmetcalf, netdev
In-Reply-To: <20110112.234250.10542369.davem@davemloft.net>
On 2011-01-13 at 08:42:50 +0100, David Miller <davem@davemloft.net> wrote:
> From: Tobias Klauser <tklauser@distanz.ch>
> Date: Thu, 13 Jan 2011 07:58:55 +0100
>
> > On 2011-01-13 at 03:45:01 +0100, David Miller <davem@davemloft.net> wrote:
> >> From: Chris Metcalf <cmetcalf@tilera.com>
> >> Date: Wed, 12 Jan 2011 12:49:03 -0500
> >>
> >> > On 1/12/2011 4:31 AM, Tobias Klauser wrote:
> >> >> Use is_multicast_ether_addr from linux/etherdevice.h instead of a custom
> >> >> macro. Also remove the broadcast address check, as it is considered a
> >> >> multicast address too.
> >> >>
> >> >> Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> >> >> ---
> >> >> drivers/net/tile/tilepro.c | 10 +---------
> >> >> 1 files changed, 1 insertions(+), 9 deletions(-)
> >> >
> >> > Thanks, I've taken this into the Tilera tree!
> >>
> >> Don't, his transformation is buggy.
> >
> > Why?
> >
> > Doesn't the code want to make sure that only unicast addresses get
> > filtered?
> >
> >> You can't get rid of the broadcast check, it needs to be there.
> >> Think about it.
> >
> > If a unicast address is in buf, is_multicast_ether_addr returns false
> > (and is_broadcast_addr would too) and thus it would get filtered.
>
> Ok, this may be correct, but it makes the code hard to read.
>
> I think the old code, whilst redundant, is easier to understand.
Agreed. Thanks for clarifying.
> Why not add a function "is_unicast_ether_addr()" and use that
> here instead?
I'll send a patch to add the function to linux/etherdevice.h and an
updated patch for the tilepro driver as a follow up.
^ permalink raw reply
* [PATCH net-next-2.6] etherdevice.h: Add is_unicast_ether_addr function
From: Tobias Klauser @ 2011-01-13 8:14 UTC (permalink / raw)
To: David Miller, cmetcalf, netdev; +Cc: tklauser
In-Reply-To: <20110112.234250.10542369.davem@davemloft.net>
>From a check for !is_multicast_ether_addr it is not always obvious that
we're checking for a unicast address. So add this helper function to
make those code paths easier to read.
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
include/linux/etherdevice.h | 11 +++++++++++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
index bec8b82..ab68f78 100644
--- a/include/linux/etherdevice.h
+++ b/include/linux/etherdevice.h
@@ -99,6 +99,17 @@ static inline int is_broadcast_ether_addr(const u8 *addr)
}
/**
+ * is_unicast_ether_addr - Determine if the Ethernet address is unicast
+ * @addr: Pointer to a six-byte array containing the Ethernet address
+ *
+ * Return true if the address is a unicast address.
+ */
+static inline int is_unicast_ether_addr(const u8 *addr)
+{
+ return !is_multicast_ether_addr(addr);
+}
+
+/**
* is_valid_ether_addr - Determine if the given Ethernet address is valid
* @addr: Pointer to a six-byte array containing the Ethernet address
*
--
1.7.0.4
^ permalink raw reply related
* [PATCH v2 net-next-2.6] netdev: tilepro: Use is_unicast_ether_addr helper
From: Tobias Klauser @ 2011-01-13 8:15 UTC (permalink / raw)
To: David Miller, cmetcalf, netdev; +Cc: tklauser
In-Reply-To: <20110112.234250.10542369.davem@davemloft.net>
Use is_unicast_ether_addr from linux/etherdevice.h instead of custom
macros.
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
drivers/net/tile/tilepro.c | 10 +---------
1 files changed, 1 insertions(+), 9 deletions(-)
diff --git a/drivers/net/tile/tilepro.c b/drivers/net/tile/tilepro.c
index 0e6bac5..7cb301d 100644
--- a/drivers/net/tile/tilepro.c
+++ b/drivers/net/tile/tilepro.c
@@ -142,14 +142,6 @@
MODULE_AUTHOR("Tilera");
MODULE_LICENSE("GPL");
-
-#define IS_MULTICAST(mac_addr) \
- (((u8 *)(mac_addr))[0] & 0x01)
-
-#define IS_BROADCAST(mac_addr) \
- (((u16 *)(mac_addr))[0] == 0xffff)
-
-
/*
* Queue of incoming packets for a specific cpu and device.
*
@@ -795,7 +787,7 @@ static bool tile_net_poll_aux(struct tile_net_cpu *info, int index)
/*
* FIXME: Implement HW multicast filter.
*/
- if (!IS_MULTICAST(buf) && !IS_BROADCAST(buf)) {
+ if (is_unicast_ether_addr(buf)) {
/* Filter packets not for our address. */
const u8 *mine = dev->dev_addr;
filter = compare_ether_addr(mine, buf);
--
1.7.0.4
^ permalink raw reply related
* Re: [patch 2/2] [PATCH] qeth: l3 hw tx csum circumvent hw bug
From: Frank Blaschka @ 2011-01-13 8:23 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-s390
In-Reply-To: <20110112.234735.160088590.davem@davemloft.net>
On Wed, Jan 12, 2011 at 11:47:35PM -0800, David Miller wrote:
> From: frank.blaschka@de.ibm.com
> Date: Thu, 13 Jan 2011 07:42:25 +0100
>
> > --- a/drivers/s390/net/qeth_l3_main.c
> > +++ b/drivers/s390/net/qeth_l3_main.c
> > @@ -2998,7 +2998,9 @@ static inline void qeth_l3_hdr_csum(stru
> > */
> > if (iph->protocol == IPPROTO_UDP)
> > hdr->hdr.l3.ext_flags |= QETH_HDR_EXT_UDP;
> > - hdr->hdr.l3.ext_flags |= QETH_HDR_EXT_CSUM_TRANSP_REQ;
> > + hdr->hdr.l3.ext_flags |= QETH_HDR_EXT_CSUM_TRANSP_REQ |
> > + QETH_HDR_EXT_CSUM_HDR_REQ;
> > + iph->check = 0;
> > if (card->options.performance_stats)
> > card->perf_stats.tx_csum++;
> > }
>
> You may not change the packet header contents blindly like this.
> Otherwise unpredictable contents will be seen by tcpdump and any
> other code path which has a clone of this packet.
>
> Thus, you'll need to guard this change with something like:
>
> if (skb_header_cloned(skb) &&
> pskb_expand_head(skb, 0, 0, GFP_ATOMIC)) {
> dev_kfree_skb(skb);
> goto tx_fail;
> }
Yes I know. Because of the suboptimal l3 driver design :-) we already have
a private copy of the skb at this place. Thx!
>
> Please fix this and resubmit your two patches.
> --
> 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: [PATCH net-next-2.6] etherdevice.h: Add is_unicast_ether_addr function
From: Joe Perches @ 2011-01-13 8:24 UTC (permalink / raw)
To: Tobias Klauser; +Cc: David Miller, cmetcalf, netdev
In-Reply-To: <1294906496-14950-1-git-send-email-tklauser@distanz.ch>
On Thu, 2011-01-13 at 09:14 +0100, Tobias Klauser wrote:
> >From a check for !is_multicast_ether_addr it is not always obvious that
> we're checking for a unicast address. So add this helper function to
> make those code paths easier to read.
> include/linux/etherdevice.h | 11 +++++++++++
[]
> /**
> + * is_unicast_ether_addr - Determine if the Ethernet address is unicast
> + * @addr: Pointer to a six-byte array containing the Ethernet address
> + *
> + * Return true if the address is a unicast address.
> + */
> +static inline int is_unicast_ether_addr(const u8 *addr)
> +{
> + return !is_multicast_ether_addr(addr);
> +}
Can't you simply use is_valid_ether_addr?
I can't think of much reason that a new function for
!multicast without the !is_zero is needed.
^ permalink raw reply
* Re: [PATCH net-next-2.6] etherdevice.h: Add is_unicast_ether_addr function
From: Tobias Klauser @ 2011-01-13 8:35 UTC (permalink / raw)
To: Joe Perches; +Cc: David Miller, cmetcalf, netdev
In-Reply-To: <1294907081.4114.35.camel@Joe-Laptop>
On 2011-01-13 at 09:24:41 +0100, Joe Perches <joe@perches.com> wrote:
> On Thu, 2011-01-13 at 09:14 +0100, Tobias Klauser wrote:
> > >From a check for !is_multicast_ether_addr it is not always obvious that
> > we're checking for a unicast address. So add this helper function to
> > make those code paths easier to read.
> > include/linux/etherdevice.h | 11 +++++++++++
> []
> > /**
> > + * is_unicast_ether_addr - Determine if the Ethernet address is unicast
> > + * @addr: Pointer to a six-byte array containing the Ethernet address
> > + *
> > + * Return true if the address is a unicast address.
> > + */
> > +static inline int is_unicast_ether_addr(const u8 *addr)
> > +{
> > + return !is_multicast_ether_addr(addr);
> > +}
>
> Can't you simply use is_valid_ether_addr?
I added is_unicast_ether_addr to make the intention of checking for a
unicast address clearer (see [1] for context).
[1] http://article.gmane.org/gmane.linux.network/183615
> I can't think of much reason that a new function for
> !multicast without the !is_zero is needed.
Maybe I should just add something like the following?
#define is_unicast_ether_addr(addr) is_valid_ether_addr(addr)
^ permalink raw reply
* Re: [PATCH net-next-2.6] etherdevice.h: Add is_unicast_ether_addr function
From: Joe Perches @ 2011-01-13 8:43 UTC (permalink / raw)
To: Tobias Klauser; +Cc: David Miller, cmetcalf, netdev
In-Reply-To: <20110113083519.GW29757@distanz.ch>
On Thu, 2011-01-13 at 09:35 +0100, Tobias Klauser wrote:
> On 2011-01-13 at 09:24:41 +0100, Joe Perches <joe@perches.com> wrote:
> I added is_unicast_ether_addr to make the intention of checking for a
> unicast address clearer (see [1] for context).
> [1] http://article.gmane.org/gmane.linux.network/183615
> > I can't think of much reason that a new function for
> > !multicast without the !is_zero is needed.
> Maybe I should just add something like the following?
> #define is_unicast_ether_addr(addr) is_valid_ether_addr(addr)
Adding the #define makes a bit more sense to me.
^ permalink raw reply
* Re: [PATCH net-next-2.6] etherdevice.h: Add is_unicast_ether_addr function
From: Eric Dumazet @ 2011-01-13 8:55 UTC (permalink / raw)
To: Joe Perches; +Cc: Tobias Klauser, David Miller, cmetcalf, netdev
In-Reply-To: <1294907081.4114.35.camel@Joe-Laptop>
Le jeudi 13 janvier 2011 à 00:24 -0800, Joe Perches a écrit :
> On Thu, 2011-01-13 at 09:14 +0100, Tobias Klauser wrote:
> > >From a check for !is_multicast_ether_addr it is not always obvious that
> > we're checking for a unicast address. So add this helper function to
> > make those code paths easier to read.
> > include/linux/etherdevice.h | 11 +++++++++++
> []
> > /**
> > + * is_unicast_ether_addr - Determine if the Ethernet address is unicast
> > + * @addr: Pointer to a six-byte array containing the Ethernet address
> > + *
> > + * Return true if the address is a unicast address.
> > + */
> > +static inline int is_unicast_ether_addr(const u8 *addr)
> > +{
> > + return !is_multicast_ether_addr(addr);
> > +}
>
> Can't you simply use is_valid_ether_addr?
>
> I can't think of much reason that a new function for
> !multicast without the !is_zero is needed.
>
performance ?
is_valid_ether_addr() is used at device init time, not when receiving
packets.
I am not sure we _need_ to check for is_zero_ether_addr() each time we
receive a packet.
Either a MAC is unicast or multicast.
A zero address is not multicast for sure.
^ permalink raw reply
* [PATCH 01/10] GRETH: added raw AMBA vendor/device number to match against.
From: Daniel Hellstrom @ 2011-01-13 8:25 UTC (permalink / raw)
To: davem; +Cc: netdev, kristoffer
Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
---
drivers/net/greth.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/net/greth.c b/drivers/net/greth.c
index 27d6960..1c2dbdb 100644
--- a/drivers/net/greth.c
+++ b/drivers/net/greth.c
@@ -1600,6 +1600,9 @@ static struct of_device_id greth_of_match[] = {
{
.name = "GAISLER_ETHMAC",
},
+ {
+ .name = "01_01d",
+ },
{},
};
--
1.5.4
^ permalink raw reply related
* [PATCH 02/10] GRETH: added option to disable a device node from bootloader.
From: Daniel Hellstrom @ 2011-01-13 8:25 UTC (permalink / raw)
To: davem; +Cc: netdev, kristoffer
In-Reply-To: <1294907135-24884-1-git-send-email-daniel@gaisler.com>
Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
---
drivers/net/greth.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/drivers/net/greth.c b/drivers/net/greth.c
index 1c2dbdb..1b10186 100644
--- a/drivers/net/greth.c
+++ b/drivers/net/greth.c
@@ -1383,6 +1383,12 @@ static int __devinit greth_of_probe(struct platform_device *ofdev, const struct
int err;
int tmp;
unsigned long timeout;
+ int *ampopts;
+
+ /* Skip device if used by another OS instance */
+ ampopts = (int *) of_get_property(ofdev->dev.of_node, "ampopts", NULL);
+ if (ampopts && (*ampopts == 0))
+ return -EIO;
dev = alloc_etherdev(sizeof(struct greth_private));
--
1.5.4
^ permalink raw reply related
* [PATCH 05/10] GRETH: fix opening/closing
From: Daniel Hellstrom @ 2011-01-13 8:25 UTC (permalink / raw)
To: davem; +Cc: netdev, kristoffer
In-Reply-To: <1294907135-24884-1-git-send-email-daniel@gaisler.com>
When NAPI is disabled there is no point in having IRQs enabled, TX/RX
should be off before clearing the TX/RX descriptor rings.
Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
---
drivers/net/greth.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/drivers/net/greth.c b/drivers/net/greth.c
index 775dc24..27578c9 100644
--- a/drivers/net/greth.c
+++ b/drivers/net/greth.c
@@ -366,6 +366,8 @@ static int greth_open(struct net_device *dev)
dev_dbg(&dev->dev, " starting queue\n");
netif_start_queue(dev);
+ GRETH_REGSAVE(greth->regs->status, 0xFF);
+
napi_enable(&greth->napi);
greth_enable_irqs(greth);
@@ -381,7 +383,9 @@ static int greth_close(struct net_device *dev)
napi_disable(&greth->napi);
+ greth_disable_irqs(greth);
greth_disable_tx(greth);
+ greth_disable_rx(greth);
netif_stop_queue(dev);
--
1.5.4
^ permalink raw reply related
* [PATCH 08/10] GRETH: avoid writing bad speed/duplex when setting transfer mode
From: Daniel Hellstrom @ 2011-01-13 8:25 UTC (permalink / raw)
To: davem; +Cc: netdev, kristoffer
In-Reply-To: <1294907135-24884-1-git-send-email-daniel@gaisler.com>
Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
---
drivers/net/greth.c | 19 ++++++++-----------
1 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/drivers/net/greth.c b/drivers/net/greth.c
index 888dc65..fea1e20 100644
--- a/drivers/net/greth.c
+++ b/drivers/net/greth.c
@@ -1242,29 +1242,26 @@ static void greth_link_change(struct net_device *dev)
struct greth_private *greth = netdev_priv(dev);
struct phy_device *phydev = greth->phy;
unsigned long flags;
-
int status_change = 0;
+ u32 ctrl;
spin_lock_irqsave(&greth->devlock, flags);
if (phydev->link) {
if ((greth->speed != phydev->speed) || (greth->duplex != phydev->duplex)) {
-
- GRETH_REGANDIN(greth->regs->control,
- ~(GRETH_CTRL_FD | GRETH_CTRL_SP | GRETH_CTRL_GB));
+ ctrl = GRETH_REGLOAD(greth->regs->control) &
+ ~(GRETH_CTRL_FD | GRETH_CTRL_SP | GRETH_CTRL_GB);
if (phydev->duplex)
- GRETH_REGORIN(greth->regs->control, GRETH_CTRL_FD);
-
- if (phydev->speed == SPEED_100) {
-
- GRETH_REGORIN(greth->regs->control, GRETH_CTRL_SP);
- }
+ ctrl |= GRETH_CTRL_FD;
+ if (phydev->speed == SPEED_100)
+ ctrl |= GRETH_CTRL_SP;
else if (phydev->speed == SPEED_1000)
- GRETH_REGORIN(greth->regs->control, GRETH_CTRL_GB);
+ ctrl |= GRETH_CTRL_GB;
+ GRETH_REGSAVE(greth->regs->control, ctrl);
greth->speed = phydev->speed;
greth->duplex = phydev->duplex;
status_change = 1;
--
1.5.4
^ permalink raw reply related
* [PATCH 04/10] GRETH: added greth_compat_mode module parameter
From: Daniel Hellstrom @ 2011-01-13 8:25 UTC (permalink / raw)
To: davem; +Cc: netdev, kristoffer
In-Reply-To: <1294907135-24884-1-git-send-email-daniel@gaisler.com>
The greth_compat_mode option can be used to set a GRETH GBit capable MAC
in operate as if the GRETH 10/100 device was found. The GRETH GBit supports
TCP/UDP checksum offloading, unaligned frame buffers, scatter gather etc.
Enabling this mode allows the developer to test the GRETH 10/100 device
without all features mentioned above on a GBit MAC capable of the above.
Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
---
drivers/net/greth.c | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/drivers/net/greth.c b/drivers/net/greth.c
index ef8da22..775dc24 100644
--- a/drivers/net/greth.c
+++ b/drivers/net/greth.c
@@ -70,6 +70,12 @@ static int no_gbit = 0;
module_param(no_gbit, int, S_IRUGO);
MODULE_PARM_DESC(no_gbit, "GRETH reports only 10/100 support to PHY layer if set to 1. Only affects GRETH GBit MAC, default 0 (off).");
+/* Use this option to enable GRETH 10/100 code on GRETH_GBIT hardware
+ * (debug legacy code option) */
+static int compat_mode = 0;
+module_param(compat_mode, int, S_IRUGO);
+MODULE_PARM_DESC(compat_mode, "GRETH 10/100 legacy mode enable. Only affects GRETH GBit MAC, default 0 (off).");
+
static int greth_open(struct net_device *dev);
static netdev_tx_t greth_start_xmit(struct sk_buff *skb,
struct net_device *dev);
@@ -1458,6 +1464,10 @@ static int __devinit greth_of_probe(struct platform_device *ofdev, const struct
else
greth->gbit_phy_support = 0;
+ /* Force GBit MAC in legacy 10/100 mode (no offloading etc.) */
+ if (compat_mode == 1)
+ greth->gbit_mac = 0;
+
/* Check for multicast capability */
greth->multicast = (tmp >> 25) & 1;
--
1.5.4
^ permalink raw reply related
* [PATCH 09/10] GRETH: handle frame error interrupts
From: Daniel Hellstrom @ 2011-01-13 8:25 UTC (permalink / raw)
To: davem; +Cc: netdev, kristoffer
In-Reply-To: <1294907135-24884-1-git-send-email-daniel@gaisler.com>
Frame error interrupts must also be handled since the RX flag only indicates
successful reception, it is unlikely but the old code may lead to dead lock
if 128 error frames are recieved in a row.
Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
---
drivers/net/greth.c | 9 +++++----
drivers/net/greth.h | 2 ++
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/net/greth.c b/drivers/net/greth.c
index fea1e20..b9623d2 100644
--- a/drivers/net/greth.c
+++ b/drivers/net/greth.c
@@ -596,12 +596,13 @@ static irqreturn_t greth_interrupt(int irq, void *dev_id)
status = GRETH_REGLOAD(greth->regs->status);
/* Handle rx and tx interrupts through poll */
- if (status & (GRETH_INT_RX | GRETH_INT_TX)) {
+ if (status & (GRETH_INT_RE | GRETH_INT_RX |
+ GRETH_INT_TE | GRETH_INT_TX)) {
/* Clear interrupt status */
- GRETH_REGORIN(greth->regs->status,
- status & (GRETH_INT_RX | GRETH_INT_TX));
-
+ GRETH_REGSAVE(greth->regs->status,
+ status & (GRETH_INT_RE | GRETH_INT_RX |
+ GRETH_INT_TE | GRETH_INT_TX));
retval = IRQ_HANDLED;
/* Disable interrupts and schedule poll() */
diff --git a/drivers/net/greth.h b/drivers/net/greth.h
index 9414169..f97f553 100644
--- a/drivers/net/greth.h
+++ b/drivers/net/greth.h
@@ -23,6 +23,7 @@
#define GRETH_BD_LEN 0x7FF
#define GRETH_TXEN 0x1
+#define GRETH_INT_TE 0x2
#define GRETH_INT_TX 0x8
#define GRETH_TXI 0x4
#define GRETH_TXBD_STATUS 0x0001C000
@@ -35,6 +36,7 @@
#define GRETH_TXBD_ERR_UE 0x4000
#define GRETH_TXBD_ERR_AL 0x8000
+#define GRETH_INT_RE 0x1
#define GRETH_INT_RX 0x4
#define GRETH_RXEN 0x2
#define GRETH_RXI 0x8
--
1.5.4
^ permalink raw reply related
* [PATCH 07/10] GRETH: fixed skb buffer memory leak on frame errors
From: Daniel Hellstrom @ 2011-01-13 8:25 UTC (permalink / raw)
To: davem; +Cc: netdev, kristoffer
In-Reply-To: <1294907135-24884-1-git-send-email-daniel@gaisler.com>
A new SKB buffer should not be allocated when the old SKB is reused.
Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
---
drivers/net/greth.c | 18 ++++++++++++++----
1 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/net/greth.c b/drivers/net/greth.c
index 72a4317..888dc65 100644
--- a/drivers/net/greth.c
+++ b/drivers/net/greth.c
@@ -880,10 +880,9 @@ static int greth_rx_gbit(struct net_device *dev, int limit)
}
}
- /* Allocate new skb to replace current */
- newskb = netdev_alloc_skb(dev, MAX_FRAME_SIZE + NET_IP_ALIGN);
-
- if (!bad && newskb) {
+ /* Allocate new skb to replace current, not needed if the
+ * current skb can be reused */
+ if (!bad && (newskb=netdev_alloc_skb(dev, MAX_FRAME_SIZE + NET_IP_ALIGN))) {
skb_reserve(newskb, NET_IP_ALIGN);
dma_addr = dma_map_single(greth->dev,
@@ -920,11 +919,22 @@ static int greth_rx_gbit(struct net_device *dev, int limit)
if (net_ratelimit())
dev_warn(greth->dev, "Could not create DMA mapping, dropping packet\n");
dev_kfree_skb(newskb);
+ /* reusing current skb, so it is a drop */
dev->stats.rx_dropped++;
}
+ } else if (bad) {
+ /* Bad Frame transfer, the skb is reused */
+ dev->stats.rx_dropped++;
} else {
+ /* Failed Allocating a new skb. This is rather stupid
+ * but the current "filled" skb is reused, as if
+ * transfer failure. One could argue that RX descriptor
+ * table handling should be divided into cleaning and
+ * filling as the TX part of the driver
+ */
if (net_ratelimit())
dev_warn(greth->dev, "Could not allocate SKB, dropping packet\n");
+ /* reusing current skb, so it is a drop */
dev->stats.rx_dropped++;
}
--
1.5.4
^ permalink raw reply related
* [PATCH 06/10] GRETH: GBit transmit descriptor handling optimization
From: Daniel Hellstrom @ 2011-01-13 8:25 UTC (permalink / raw)
To: davem; +Cc: netdev, kristoffer
In-Reply-To: <1294907135-24884-1-git-send-email-daniel@gaisler.com>
It is safe to enable all fragments before enabling the first descriptor,
this way all descriptors don't have to be processed twice, added extra
memory barrier.
Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
---
drivers/net/greth.c | 19 ++++++++++---------
1 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/net/greth.c b/drivers/net/greth.c
index 27578c9..72a4317 100644
--- a/drivers/net/greth.c
+++ b/drivers/net/greth.c
@@ -513,7 +513,7 @@ greth_start_xmit_gbit(struct sk_buff *skb, struct net_device *dev)
greth->tx_skbuff[curr_tx] = NULL;
bdp = greth->tx_bd_base + curr_tx;
- status = GRETH_TXBD_CSALL;
+ status = GRETH_TXBD_CSALL | GRETH_BD_EN;
status |= frag->size & GRETH_BD_LEN;
/* Wrap around descriptor ring */
@@ -550,26 +550,27 @@ greth_start_xmit_gbit(struct sk_buff *skb, struct net_device *dev)
wmb();
- /* Enable the descriptors that we configured ... */
- for (i = 0; i < nr_frags + 1; i++) {
- bdp = greth->tx_bd_base + greth->tx_next;
- greth_write_bd(&bdp->stat, greth_read_bd(&bdp->stat) | GRETH_BD_EN);
- greth->tx_next = NEXT_TX(greth->tx_next);
- greth->tx_free--;
- }
+ /* Enable the descriptor chain by enabling the first descriptor */
+ bdp = greth->tx_bd_base + greth->tx_next;
+ greth_write_bd(&bdp->stat, greth_read_bd(&bdp->stat) | GRETH_BD_EN);
+ greth->tx_next = curr_tx;
+ greth->tx_free -= nr_frags + 1;
+
+ wmb();
greth_enable_tx(greth);
return NETDEV_TX_OK;
frag_map_error:
- /* Unmap SKB mappings that succeeded */
+ /* Unmap SKB mappings that succeeded and disable descriptor */
for (i = 0; greth->tx_next + i != curr_tx; i++) {
bdp = greth->tx_bd_base + greth->tx_next + i;
dma_unmap_single(greth->dev,
greth_read_bd(&bdp->addr),
greth_read_bd(&bdp->stat) & GRETH_BD_LEN,
DMA_TO_DEVICE);
+ greth_write_bd(&bdp->stat, 0);
}
map_error:
if (net_ratelimit())
--
1.5.4
^ permalink raw reply related
* [PATCH 03/10] GRETH: added no_gbit option
From: Daniel Hellstrom @ 2011-01-13 8:25 UTC (permalink / raw)
To: davem; +Cc: netdev, kristoffer
In-Reply-To: <1294907135-24884-1-git-send-email-daniel@gaisler.com>
For debug only. The driver does not report that it is GBit capable, instead
it will report 10/100 mode to the generic PHY layer.
Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
---
drivers/net/greth.c | 15 +++++++++++++--
drivers/net/greth.h | 1 +
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/net/greth.c b/drivers/net/greth.c
index 1b10186..ef8da22 100644
--- a/drivers/net/greth.c
+++ b/drivers/net/greth.c
@@ -66,6 +66,10 @@ static int greth_edcl = 1;
module_param(greth_edcl, int, 0);
MODULE_PARM_DESC(greth_edcl, "GRETH EDCL usage indicator. Set to 1 if EDCL is used.");
+static int no_gbit = 0;
+module_param(no_gbit, int, S_IRUGO);
+MODULE_PARM_DESC(no_gbit, "GRETH reports only 10/100 support to PHY layer if set to 1. Only affects GRETH GBit MAC, default 0 (off).");
+
static int greth_open(struct net_device *dev);
static netdev_tx_t greth_start_xmit(struct sk_buff *skb,
struct net_device *dev);
@@ -1284,7 +1288,7 @@ static int greth_mdio_probe(struct net_device *dev)
}
ret = phy_connect_direct(dev, phy, &greth_link_change,
- 0, greth->gbit_mac ?
+ 0, greth->gbit_phy_support ?
PHY_INTERFACE_MODE_GMII :
PHY_INTERFACE_MODE_MII);
if (ret) {
@@ -1293,7 +1297,7 @@ static int greth_mdio_probe(struct net_device *dev)
return ret;
}
- if (greth->gbit_mac)
+ if (greth->gbit_phy_support)
phy->supported &= PHY_GBIT_FEATURES;
else
phy->supported &= PHY_BASIC_FEATURES;
@@ -1447,6 +1451,13 @@ static int __devinit greth_of_probe(struct platform_device *ofdev, const struct
tmp = GRETH_REGLOAD(regs->control);
greth->gbit_mac = (tmp >> 27) & 1;
+ /* Let user skip GBit link mode by telling MDIO layer that MAC does
+ * not support GBIT (for debug) */
+ if (greth->gbit_mac && !no_gbit)
+ greth->gbit_phy_support = 1;
+ else
+ greth->gbit_phy_support = 0;
+
/* Check for multicast capability */
greth->multicast = (tmp >> 25) & 1;
diff --git a/drivers/net/greth.h b/drivers/net/greth.h
index 03ad903..9414169 100644
--- a/drivers/net/greth.h
+++ b/drivers/net/greth.h
@@ -138,6 +138,7 @@ struct greth_private {
u8 gbit_mac;
u8 mdio_int_en;
u8 edcl;
+ u8 gbit_phy_support;
};
#endif
--
1.5.4
^ permalink raw reply related
* [PATCH 10/10] GRETH: resolve SMP issues and other problems
From: Daniel Hellstrom @ 2011-01-13 8:25 UTC (permalink / raw)
To: davem; +Cc: netdev, kristoffer
In-Reply-To: <1294907135-24884-1-git-send-email-daniel@gaisler.com>
Fixes the following:
1. POLL should not enable IRQ when work is not completed
2. No locking between TX descriptor cleaning and XMIT descriptor handling
3. No locking between RX POLL and XMIT modifying control register
4. Since TX cleaning (called from POLL) is running in parallel with XMIT
unnecessary locking is needed.
5. IRQ handler looks at RX frame status solely, this is wrong when IRQ is
temporarily disabled (in POLL), and when IRQ is shared.
6. IRQ handler clears IRQ status, which is unnecessary
7. TX queue was stopped in preventing cause when not MAX_SKB_FRAGS+1
descriptors were available after a SKB been scheduled by XMIT. Instead
the TX queue is stopped first when not enough descriptors are available
upon entering XMIT.
It was hard to split up this patch in smaller pieces since all are tied
together somehow.
Note the RX flag used in the interrupt handler does not signal that
interrupt was asserted, but that a frame was received. Same goes for TX.
Also, IRQ is not asserted when the RX flag is set before enabling IRQ
enable until a new frame is received. So extra care must be taken to
avoid enabling IRQ and all descriptors are already used, hence dead lock
will upon us. See new POLL implementation that enableds IRQ then look at
the RX flag to determine if one or more IRQs may have been missed. TX/RX
flags are cleared before handling previously enabled descriptors, this
ensures that the RX/TX flags are valid when determining if IRQ should be
turned on again.
By moving TX cleaning from POLL to XMIT in the standard case, removes some
locking trouble. Enabling TX cleaning from poll only when not enough TX
descriptors are available is safe because the TX queue is at the same time
stopped, thus XMIT will not be called. The TX queue is woken up again when
enough descriptrs are available.
TX Frames are always enabled with IRQ, however the TX IRQ Enable flag will
not be enabled until XMIT must wait for free descriptors.
Locking RX and XMIT parts of the driver from each other is needed because
the RX/TX enable bits share the same register.
Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
---
drivers/net/greth.c | 159 +++++++++++++++++++++++++++++---------------------
1 files changed, 92 insertions(+), 67 deletions(-)
diff --git a/drivers/net/greth.c b/drivers/net/greth.c
index b9623d2..954f65a 100644
--- a/drivers/net/greth.c
+++ b/drivers/net/greth.c
@@ -1,7 +1,7 @@
/*
* Aeroflex Gaisler GRETH 10/100/1G Ethernet MAC.
*
- * 2005-2009 (c) Aeroflex Gaisler AB
+ * 2005-2010 (c) Aeroflex Gaisler AB
*
* This driver supports GRETH 10/100 and GRETH 10/100/1G Ethernet MACs
* available in the GRLIB VHDL IP core library.
@@ -402,12 +402,20 @@ greth_start_xmit(struct sk_buff *skb, struct net_device *dev)
struct greth_private *greth = netdev_priv(dev);
struct greth_bd *bdp;
int err = NETDEV_TX_OK;
- u32 status, dma_addr;
+ u32 status, dma_addr, ctrl;
+ unsigned long flags;
- bdp = greth->tx_bd_base + greth->tx_next;
+ /* Clean TX Ring */
+ greth_clean_tx(greth->netdev);
if (unlikely(greth->tx_free <= 0)) {
+ spin_lock_irqsave(&greth->devlock, flags);/*save from poll/irq*/
+ ctrl = GRETH_REGLOAD(greth->regs->control);
+ /* Enable TX IRQ only if not already in poll() routine */
+ if (ctrl & GRETH_RXI)
+ GRETH_REGSAVE(greth->regs->control, ctrl | GRETH_TXI);
netif_stop_queue(dev);
+ spin_unlock_irqrestore(&greth->devlock, flags);
return NETDEV_TX_BUSY;
}
@@ -420,13 +428,14 @@ greth_start_xmit(struct sk_buff *skb, struct net_device *dev)
goto out;
}
+ bdp = greth->tx_bd_base + greth->tx_next;
dma_addr = greth_read_bd(&bdp->addr);
memcpy((unsigned char *) phys_to_virt(dma_addr), skb->data, skb->len);
dma_sync_single_for_device(greth->dev, dma_addr, skb->len, DMA_TO_DEVICE);
- status = GRETH_BD_EN | (skb->len & GRETH_BD_LEN);
+ status = GRETH_BD_EN | GRETH_BD_IE | (skb->len & GRETH_BD_LEN);
/* Wrap around descriptor ring */
if (greth->tx_next == GRETH_TXBD_NUM_MASK) {
@@ -436,22 +445,11 @@ greth_start_xmit(struct sk_buff *skb, struct net_device *dev)
greth->tx_next = NEXT_TX(greth->tx_next);
greth->tx_free--;
- /* No more descriptors */
- if (unlikely(greth->tx_free == 0)) {
-
- /* Free transmitted descriptors */
- greth_clean_tx(dev);
-
- /* If nothing was cleaned, stop queue & wait for irq */
- if (unlikely(greth->tx_free == 0)) {
- status |= GRETH_BD_IE;
- netif_stop_queue(dev);
- }
- }
-
/* Write descriptor control word and enable transmission */
greth_write_bd(&bdp->stat, status);
+ spin_lock_irqsave(&greth->devlock, flags); /*save from poll/irq*/
greth_enable_tx(greth);
+ spin_unlock_irqrestore(&greth->devlock, flags);
out:
dev_kfree_skb(skb);
@@ -464,13 +462,23 @@ greth_start_xmit_gbit(struct sk_buff *skb, struct net_device *dev)
{
struct greth_private *greth = netdev_priv(dev);
struct greth_bd *bdp;
- u32 status = 0, dma_addr;
+ u32 status = 0, dma_addr, ctrl;
int curr_tx, nr_frags, i, err = NETDEV_TX_OK;
+ unsigned long flags;
nr_frags = skb_shinfo(skb)->nr_frags;
+ /* Clean TX Ring */
+ greth_clean_tx_gbit(dev);
+
if (greth->tx_free < nr_frags + 1) {
+ spin_lock_irqsave(&greth->devlock, flags);/*save from poll/irq*/
+ ctrl = GRETH_REGLOAD(greth->regs->control);
+ /* Enable TX IRQ only if not already in poll() routine */
+ if (ctrl & GRETH_RXI)
+ GRETH_REGSAVE(greth->regs->control, ctrl | GRETH_TXI);
netif_stop_queue(dev);
+ spin_unlock_irqrestore(&greth->devlock, flags);
err = NETDEV_TX_BUSY;
goto out;
}
@@ -523,14 +531,8 @@ greth_start_xmit_gbit(struct sk_buff *skb, struct net_device *dev)
/* More fragments left */
if (i < nr_frags - 1)
status |= GRETH_TXBD_MORE;
-
- /* ... last fragment, check if out of descriptors */
- else if (greth->tx_free - nr_frags - 1 < (MAX_SKB_FRAGS + 1)) {
-
- /* Enable interrupts and stop queue */
- status |= GRETH_BD_IE;
- netif_stop_queue(dev);
- }
+ else
+ status |= GRETH_BD_IE; /* enable IRQ on last fragment */
greth_write_bd(&bdp->stat, status);
@@ -558,7 +560,9 @@ greth_start_xmit_gbit(struct sk_buff *skb, struct net_device *dev)
wmb();
+ spin_lock_irqsave(&greth->devlock, flags); /*save from poll/irq*/
greth_enable_tx(greth);
+ spin_unlock_irqrestore(&greth->devlock, flags);
return NETDEV_TX_OK;
@@ -580,12 +584,11 @@ out:
return err;
}
-
static irqreturn_t greth_interrupt(int irq, void *dev_id)
{
struct net_device *dev = dev_id;
struct greth_private *greth;
- u32 status;
+ u32 status, ctrl;
irqreturn_t retval = IRQ_NONE;
greth = netdev_priv(dev);
@@ -595,14 +598,15 @@ static irqreturn_t greth_interrupt(int irq, void *dev_id)
/* Get the interrupt events that caused us to be here. */
status = GRETH_REGLOAD(greth->regs->status);
- /* Handle rx and tx interrupts through poll */
- if (status & (GRETH_INT_RE | GRETH_INT_RX |
- GRETH_INT_TE | GRETH_INT_TX)) {
+ /* Must see if interrupts are enabled also, INT_TX|INT_RX flags may be
+ * set regardless of whether IRQ is enabled or not. Especially
+ * important when shared IRQ.
+ */
+ ctrl = GRETH_REGLOAD(greth->regs->control);
- /* Clear interrupt status */
- GRETH_REGSAVE(greth->regs->status,
- status & (GRETH_INT_RE | GRETH_INT_RX |
- GRETH_INT_TE | GRETH_INT_TX));
+ /* Handle rx and tx interrupts through poll */
+ if (((status & (GRETH_INT_RE | GRETH_INT_RX)) && (ctrl & GRETH_RXI)) ||
+ ((status & (GRETH_INT_TE | GRETH_INT_TX)) && (ctrl & GRETH_TXI))) {
retval = IRQ_HANDLED;
/* Disable interrupts and schedule poll() */
@@ -626,6 +630,8 @@ static void greth_clean_tx(struct net_device *dev)
while (1) {
bdp = greth->tx_bd_base + greth->tx_last;
+ GRETH_REGSAVE(greth->regs->status, GRETH_INT_TE | GRETH_INT_TX);
+ mb();
stat = greth_read_bd(&bdp->stat);
if (unlikely(stat & GRETH_BD_EN))
@@ -686,7 +692,10 @@ static void greth_clean_tx_gbit(struct net_device *dev)
/* We only clean fully completed SKBs */
bdp_last_frag = greth->tx_bd_base + SKIP_TX(greth->tx_last, nr_frags);
- stat = bdp_last_frag->stat;
+
+ GRETH_REGSAVE(greth->regs->status, GRETH_INT_TE | GRETH_INT_TX);
+ mb();
+ stat = greth_read_bd(&bdp_last_frag->stat);
if (stat & GRETH_BD_EN)
break;
@@ -718,21 +727,9 @@ static void greth_clean_tx_gbit(struct net_device *dev)
greth->tx_free += nr_frags+1;
dev_kfree_skb(skb);
}
- if (greth->tx_free > (MAX_SKB_FRAGS + 1)) {
- netif_wake_queue(dev);
- }
-}
-static int greth_pending_packets(struct greth_private *greth)
-{
- struct greth_bd *bdp;
- u32 status;
- bdp = greth->rx_bd_base + greth->rx_cur;
- status = greth_read_bd(&bdp->stat);
- if (status & GRETH_BD_EN)
- return 0;
- else
- return 1;
+ if (netif_queue_stopped(dev) && (greth->tx_free > (MAX_SKB_FRAGS+1)))
+ netif_wake_queue(dev);
}
static int greth_rx(struct net_device *dev, int limit)
@@ -743,20 +740,24 @@ static int greth_rx(struct net_device *dev, int limit)
int pkt_len;
int bad, count;
u32 status, dma_addr;
+ unsigned long flags;
greth = netdev_priv(dev);
for (count = 0; count < limit; ++count) {
bdp = greth->rx_bd_base + greth->rx_cur;
+ GRETH_REGSAVE(greth->regs->status, GRETH_INT_RE | GRETH_INT_RX);
+ mb();
status = greth_read_bd(&bdp->stat);
- dma_addr = greth_read_bd(&bdp->addr);
- bad = 0;
if (unlikely(status & GRETH_BD_EN)) {
break;
}
+ dma_addr = greth_read_bd(&bdp->addr);
+ bad = 0;
+
/* Check status for errors. */
if (unlikely(status & GRETH_RXBD_STATUS)) {
if (status & GRETH_RXBD_ERR_FT) {
@@ -818,7 +819,9 @@ static int greth_rx(struct net_device *dev, int limit)
dma_sync_single_for_device(greth->dev, dma_addr, MAX_FRAME_SIZE, DMA_FROM_DEVICE);
+ spin_lock_irqsave(&greth->devlock, flags); /* save from XMIT */
greth_enable_rx(greth);
+ spin_unlock_irqrestore(&greth->devlock, flags);
greth->rx_cur = NEXT_RX(greth->rx_cur);
}
@@ -852,6 +855,7 @@ static int greth_rx_gbit(struct net_device *dev, int limit)
int pkt_len;
int bad, count = 0;
u32 status, dma_addr;
+ unsigned long flags;
greth = netdev_priv(dev);
@@ -859,6 +863,8 @@ static int greth_rx_gbit(struct net_device *dev, int limit)
bdp = greth->rx_bd_base + greth->rx_cur;
skb = greth->rx_skbuff[greth->rx_cur];
+ GRETH_REGSAVE(greth->regs->status, GRETH_INT_RE | GRETH_INT_RX);
+ mb();
status = greth_read_bd(&bdp->stat);
bad = 0;
@@ -946,7 +952,9 @@ static int greth_rx_gbit(struct net_device *dev, int limit)
wmb();
greth_write_bd(&bdp->stat, status);
+ spin_lock_irqsave(&greth->devlock, flags);
greth_enable_rx(greth);
+ spin_unlock_irqrestore(&greth->devlock, flags);
greth->rx_cur = NEXT_RX(greth->rx_cur);
}
@@ -958,15 +966,18 @@ static int greth_poll(struct napi_struct *napi, int budget)
{
struct greth_private *greth;
int work_done = 0;
+ unsigned long flags;
+ u32 mask, ctrl;
greth = container_of(napi, struct greth_private, napi);
- if (greth->gbit_mac) {
- greth_clean_tx_gbit(greth->netdev);
- } else {
- greth_clean_tx(greth->netdev);
+restart_txrx_poll:
+ if (netif_queue_stopped(greth->netdev)) {
+ if (greth->gbit_mac)
+ greth_clean_tx_gbit(greth->netdev);
+ else
+ greth_clean_tx(greth->netdev);
}
-restart_poll:
if (greth->gbit_mac) {
work_done += greth_rx_gbit(greth->netdev, budget - work_done);
} else {
@@ -975,15 +986,29 @@ restart_poll:
if (work_done < budget) {
- napi_complete(napi);
+ spin_lock_irqsave(&greth->devlock, flags);
- if (greth_pending_packets(greth)) {
- napi_reschedule(napi);
- goto restart_poll;
+ ctrl = GRETH_REGLOAD(greth->regs->control);
+ if (netif_queue_stopped(greth->netdev)) {
+ GRETH_REGSAVE(greth->regs->control,
+ ctrl | GRETH_TXI | GRETH_RXI);
+ mask = GRETH_INT_RX | GRETH_INT_RE |
+ GRETH_INT_TX | GRETH_INT_TE;
+ } else {
+ GRETH_REGSAVE(greth->regs->control, ctrl | GRETH_RXI);
+ mask = GRETH_INT_RX | GRETH_INT_RE;
+ }
+
+ if (GRETH_REGLOAD(greth->regs->status) & mask) {
+ GRETH_REGSAVE(greth->regs->control, ctrl);
+ spin_unlock_irqrestore(&greth->devlock, flags);
+ goto restart_txrx_poll;
+ } else {
+ __napi_complete(napi);
+ spin_unlock_irqrestore(&greth->devlock, flags);
}
}
- greth_enable_irqs(greth);
return work_done;
}
@@ -1178,11 +1203,11 @@ static const struct ethtool_ops greth_ethtool_ops = {
};
static struct net_device_ops greth_netdev_ops = {
- .ndo_open = greth_open,
- .ndo_stop = greth_close,
- .ndo_start_xmit = greth_start_xmit,
- .ndo_set_mac_address = greth_set_mac_add,
- .ndo_validate_addr = eth_validate_addr,
+ .ndo_open = greth_open,
+ .ndo_stop = greth_close,
+ .ndo_start_xmit = greth_start_xmit,
+ .ndo_set_mac_address = greth_set_mac_add,
+ .ndo_validate_addr = eth_validate_addr,
};
static inline int wait_for_mdio(struct greth_private *greth)
--
1.5.4
^ permalink raw reply related
* RE: [E1000-devel] [e100] Page allocation failure warning(?) in 2.6.36.3
From: Chris Rankin @ 2011-01-13 9:00 UTC (permalink / raw)
To: Eric Dumazet
Cc: JesseBrandeburg, David Miller, e1000-devel@lists.sourceforge.net,
Tushar NDave, netdev@vger.kernel.org, Jeffrey TKirsher
In-Reply-To: <1294894536.3335.510.camel@edumazet-laptop>
--- On Thu, 13/1/11, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Problem is e100 allocates an order-6 page in DMA zone
> (a 256 KB contigous area of ram)
>
> This contigous area of ram is not available but just after
> booting...
I suspected as much. Fortunately, this machine has no function apart from routing and can happily left untouched for extended periods of time.
> On such small router, I doubt you need more than 64 slots
> in TX ring buffer.
But what would the effect of that change be to the interfaces' performance, please?
Cheers,
Chris
^ permalink raw reply
* RE: [E1000-devel] [e100] Page allocation failure warning(?) in 2.6.36.3
From: Eric Dumazet @ 2011-01-13 9:05 UTC (permalink / raw)
To: Chris Rankin
Cc: JesseBrandeburg, David Miller, e1000-devel@lists.sourceforge.net,
Tushar NDave, netdev@vger.kernel.org, Jeffrey TKirsher
In-Reply-To: <895344.13845.qm@web121707.mail.ne1.yahoo.com>
Le jeudi 13 janvier 2011 à 01:00 -0800, Chris Rankin a écrit :
> --- On Thu, 13/1/11, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > Problem is e100 allocates an order-6 page in DMA zone
> > (a 256 KB contigous area of ram)
> >
> > This contigous area of ram is not available but just after
> > booting...
>
> I suspected as much. Fortunately, this machine has no function apart from routing and can happily left untouched for extended periods of time.
>
> > On such small router, I doubt you need more than 64 slots
> > in TX ring buffer.
>
> But what would the effect of that change be to the interfaces' performance, please?
If you care of performance, dont unload/reload your driver all the time,
and dont use modules (this matter on old hardware because of TLB misses)
Anyway, the change ( 128 -> 64 ) is not needed, since the kernel message
is a warning only. The allocation is retried and apparently succeeds.
The __GFP_NOWARN should make the failed allocation not noticed at all.
^ permalink raw reply
* STMMAC driver: NFS Problem on 2.6.37
From: deepaksi @ 2011-01-13 9:09 UTC (permalink / raw)
To: Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
Armando VISCONTI, Shiraz HASHIM, Viresh KUMAR
Hi
I am facing a problem related to nfs boot, while using the stmmac driver
ported on 2.6.37 kernel. When we use a JFFS2 file system and mount the kernel,
the network driver works fine.
I have been following the mailing list and could find some issues with NFS
on 2.6.37 but I am not too sure whether the kernel crash I am getting is
related to that.
The driver worked fine on 2.6.32 kernel, but while booting the 2.6.37
kernel I get the following log messages:
stmmac: Rx Checksum Offload Engine supported
TX Checksum insertion supported
IP-Config: Complete:
device=eth0, addr=192.168.1.10, mask=255.255.255.0, gw=255.255.255.255,
host=192.168.1.10, domain=, nis-domain=(none),
bootserver=192.168.1.1, rootserver=192.168.1.1, rootpath=
VFS: Unable to mount root fs via NFS, trying floppy.
VFS: Cannot open root device "nfs" or unknown-block(2,0)
Please append a correct "root=" boot option; here are the available
partitions:
1f00 64 mtdblock0 (driver?)
1f01 256 mtdblock1 (driver?)
1f02 2816 mtdblock2 (driver?)
1f03 5056 mtdblock3 (driver?)
Kernel panic - not syncing: VFS: Unable to mount root fs on
unknown-block(2,0)
Backtrace:
[<c00370f0>] (dump_backtrace+0x0/0x110) from [<c0037234>]
(dump_stack+0x18/0x1c)
r7:c7b5b000 r6:00000000 r5:c7b5b015 r4:c04296b8
[<c003721c>] (dump_stack+0x0/0x1c) from [<c004ebf8>] (panic+0x60/0x180)
[<c004eb98>] (panic+0x0/0x180) from [<c0009114>]
(mount_block_root+0x1d4/0x214)
r3:00000000 r2:00000001 r1:c782bf50 r0:c0394851
[<c0008f40>] (mount_block_root+0x0/0x214) from [<c00091fc>]
(mount_root+0xa8/0xc8)
[<c0009154>] (mount_root+0x0/0xc8) from [<c0009388>]
(prepare_namespace+0x16c/0x1d0)
r4:c04288c0
[<c000921c>] (prepare_namespace+0x0/0x1d0) from [<c0008904>]
(kernel_init+0x1cc/0x220)
r5:c0402048 r4:c0428860
[<c0008738>] (kernel_init+0x0/0x220) from [<c00522a8>] (do_exit+0x0/0x5e0)
r7:00000013 r6:c00522a8 r5:c0008738 r4:00000000
CPU0: stopping
Backtrace:
[<c00370f0>] (dump_backtrace+0x0/0x110) from [<c0037234>]
(dump_stack+0x18/0x1c)
r7:c0405484 r6:00000406 r5:00000000 r4:00000000
[<c003721c>] (dump_stack+0x0/0x1c) from [<c002d334>] (do_IPI+0xb4/0x124)
[<c002d280>] (do_IPI+0x0/0x124) from [<c0032bb4>] (__irq_svc+0x34/0xc0)
Exception stack(0xc03f3f50 to 0xc03f3f98)
3f40: c0402048 00000000 c03f3f98
00000000
3f60: c03f2000 c04288dc c0027290 c0405484 000258e8 411fc091 00000000
c03f3fa4
3f80: c03f3fa8 c03f3f98 c0034a24 c0034a28 60000013 ffffffff
r5:fc800100 r4:ffffffff
[<c00349fc>] (default_idle+0x0/0x30) from [<c0034874>] (cpu_idle+0x80/0xc0)
[<c00347f4>] (cpu_idle+0x0/0xc0) from [<c030602c>] (rest_init+0x64/0x7c)
r5:c04288dc r4:c04020b0
[<c0305fc8>] (rest_init+0x0/0x7c) from [<c0008bd4>]
(start_kernel+0x27c/0x2d8)
[<c0008958>] (start_kernel+0x0/0x2d8) from [<00008038>] (0x8038)
r5:c0401fac r4:10c5387d
I have tried the same over latest source picked from linus tree,
4162cf64973df51fc885825bc9ca4d055891c49f
Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-2.6
We are using version 3 of the NFs protocol in kernel's NFS client.
Regards
Deepak
ST Microelectronics
.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: what are txqueuelen and nic ring parameters exactly?
From: Jesper Dangaard Brouer @ 2011-01-13 9:21 UTC (permalink / raw)
To: MK; +Cc: netdev
In-Reply-To: <AANLkTi=_hbmvgO9wrX6sg7Q=Wh+iVaqGdga08M-ypqae@mail.gmail.com>
On Mon, 10 Jan 2011, MK wrote:
> I often come across two variables that can be tuned for networking -
>
> 1) txqueuelen (via ifconfig )
> 2) NIC ring parameters for tx and rx (via ethtool)
>
> Can someone please tell me where these queues are exactly? Are both
> the same (seems not since their current values are different on my
> computer) . Is txqueuelen somehow part of the linux networking
> subsystem whereas the other is purely a h/w device construct?
You are spot on.
The txqueuelen is a Linux network stack thing, and is related to the
traffic control subsystem, BUT only when using the default qdisc
(pfifo_fast or mq).
If you add another qdisc, then its that specific qdiscs limits which
counts, not the device txqueuelen.
When tuning these queue lengths, you probably decrease these queue sizes,
NOT increase!
See the bufferbloat debate:
http://netoptimizer.blogspot.com/2010/12/buffer-bloat-calculations.html
http://netoptimizer.blogspot.com/2011/01/bufferbloat-wireless-is-worse-than.html
http://gettys.wordpress.com/bufferbloat-faq/
Cheers,
Jesper Brouer
--
-------------------------------------------------------------------
MSc. Master of Computer Science
Dept. of Computer Science, University of Copenhagen
Author of http://www.adsl-optimizer.dk
-------------------------------------------------------------------
^ permalink raw reply
* RE: [E1000-devel] [e100] Page allocation failure warning(?) in 2.6.36.3
From: Chris Rankin @ 2011-01-13 9:24 UTC (permalink / raw)
To: Eric Dumazet
Cc: JesseBrandeburg, David Miller, e1000-devel@lists.sourceforge.net,
Tushar NDave, netdev@vger.kernel.org, Jeffrey TKirsher
In-Reply-To: <1294909556.3570.25.camel@edumazet-laptop>
-- On Thu, 13/1/11, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> If you care of performance, dont unload/reload your driver
> all the time, and dont use modules (this matter on old hardware because
> of TLB misses)
As long as I can route the full bandwidth of my ADSLv2 connection then it's fine.
Cheers,
Chris
^ permalink raw reply
* Re: [PATCH] ipv4: devconf: start IPV4_DEVCONF_* from 0
From: Thomas Graf @ 2011-01-13 10:02 UTC (permalink / raw)
To: Lucian Adrian Grijincu
Cc: David Miller, netdev, kuznet, pekkas, jmorris, yoshfuji, kaber,
opurdila, ddvlad
In-Reply-To: <AANLkTimLyNf-sgLtzf0i3k7om9MEQ2t8tSOwDcpS66dv@mail.gmail.com>
On Thu, Jan 13, 2011 at 09:50:14AM +0200, Lucian Adrian Grijincu wrote:
> Yes it works, but there does not seem to be a good reason why to
> complicate things like this (again the sentinel nature of zero is not
> used in any place here).
The reason I didn't change anything was the same as Dave's reply, I
thought it must have been done on purpose. It probably was but I can't
spot any reason now either.
Also, IPv6 is doing just fine with using '0' as its first devconf id.
I have no objects to changing this at all but we don't gain much either.
^ 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