* Re: Re: [PATCH 2/9] xen: convert to 64 bit stats interface
From: Konrad Rzeszutek Wilk @ 2011-06-14 21:07 UTC (permalink / raw)
To: Ben Hutchings, Ian Campbell
Cc: netdev, Stephen Hemminger, xen-devel, Jeremy Fitzhardinge,
David S. Miller
In-Reply-To: <1307584605.22348.527.camel@localhost>
On Thu, Jun 09, 2011 at 02:56:45AM +0100, Ben Hutchings wrote:
> On Wed, 2011-06-08 at 17:53 -0700, Stephen Hemminger wrote:
> > Convert xen driver to 64 bit statistics interface.
> > This driver was already counting packet per queue in a 64 bit value so not
> > a huge change.
> [...]
>
> I think this driver will need to use u64_stats_sync.
Ian?
^ permalink raw reply
* RE: [RFC 2/2] ethtool: Add support for DMA Coalescing feature config to ethtool.
From: Wyborny, Carolyn @ 2011-06-14 20:19 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev@vger.kernel.org
In-Reply-To: <20110614185153.GC12002@solarflare.com>
>-----Original Message-----
>From: Ben Hutchings [mailto:bhutchings@solarflare.com]
>Sent: Tuesday, June 14, 2011 11:52 AM
>To: Wyborny, Carolyn
>Cc: netdev@vger.kernel.org
>Subject: Re: [RFC 2/2] ethtool: Add support for DMA Coalescing feature
>config to ethtool.
>
>Carolyn Wyborny wrote:
>> This RFC patch adds functions to get and set DMA Coalescing feature
>> configuration.
>>
>> Signed-off-by: Carolyn Wyborny <carolyn.wyborny@intel.com>
>> ---
>> include/linux/ethtool.h | 15 ++++++++++++++-
>> net/core/ethtool.c | 32 ++++++++++++++++++++++++++++++++
>> 2 files changed, 46 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
>> index c6a850a..efed754 100644
>> --- a/include/linux/ethtool.h
>> +++ b/include/linux/ethtool.h
>> @@ -703,6 +703,14 @@ enum ethtool_sfeatures_retval_bits {
>> ETHTOOL_F_COMPAT__BIT,
>> };
>>
>> +/* for configuring DMA coalescing parameters of chip */
>> +struct ethtool_dmac {
>> + __u32 cmd; /* ETHTOOL_{G,S}DMAC */
>> +
>> + /* tune setting, options and validation determined by device. */
>> + __u32 tune;
>> +};
>[...]
>
>I don't think we should be adding operations that have no generic
>semantics at all. Further, we will not add any operations without at
>least one implementation as an example.
>
>Secondly, ethtool operations that only get/set a 32-bit value should
>use struct ethtool_value for the parameter.
>
>Ben.
>
>--
>Ben Hutchings, Senior Software Engineer, Solarflare Communications
>Not speaking for my employer; that's the marketing department's job.
>They asked us to note that Solarflare product names are trademarked.
Ok, will send up update with the suggested changes and an implementation as an example. I have one, but will wait to synch it with the updated patch. Do you want another RFC or a regular patch submission?
Thanks,
Carolyn
Carolyn Wyborny
Linux Development
LAN Access Division
Intel Corporation
^ permalink raw reply
* [RFC][PATCH] add tracepoint to __sk_mem_schedule
From: Satoru Moriya @ 2011-06-14 19:24 UTC (permalink / raw)
To: netdev@vger.kernel.org
Cc: davem@davemloft.net, dle-develop@lists.sourceforge.net,
Seiji Aguchi
Hi,
kernel drops packets when the amount of memory which is used for socket buffer
exceeds limitations such as /proc/sys/net/ipv4/udp_mem. But currently we can't
catch that event and know why packets are dropped. And also it is difficult to
configure sysctl knob appropriately because we don't know when/why packets
dropped.
This patch adds tracepoint to __sk_mem_schedule(), which is called each time
the socket memory usage exceeds limitations and kernel drops a packet.
It allows us to hook in and examine when and why it happens.
Note that this patch only collects information which is needed for udp
because it's a RFC patch to show its concept and acutually we need it(*).
If you guys need to get other parameters, please let me know. I'll add it.
(*) Reason why we need this tracepoint for UDP
Transaction data is sent by UDP multicast in finance systems because of its
low overhead characteristics. UDP itself does not guarantee reliability,
ordering and data integrity, but the system is designed not to drop any packets
even when it is high load situation. And in that system if kernel drops packets,
we need to find a root cause to avoid it next time.
Any comments are welcome.
Signed-off-by: Satoru Moriya <satoru.moriya@hds.com>
---
include/trace/events/sock.h | 46 +++++++++++++++++++++++++++++++++++++++++++
net/core/net-traces.c | 1 +
net/core/sock.c | 4 +++
3 files changed, 51 insertions(+), 0 deletions(-)
create mode 100644 include/trace/events/sock.h
diff --git a/include/trace/events/sock.h b/include/trace/events/sock.h
new file mode 100644
index 0000000..409735a
--- /dev/null
+++ b/include/trace/events/sock.h
@@ -0,0 +1,46 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM sock
+
+#if !defined(_TRACE_SOCK_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_SOCK_H
+
+#include <net/sock.h>
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(sock_exceed_buf_limit,
+
+ TP_PROTO(struct sock *sk, struct proto *prot, long allocated),
+
+ TP_ARGS(sk, prot, allocated),
+
+ TP_STRUCT__entry(
+ __array(char, name, 32)
+ __field(long *, sysctl_mem)
+ __field(long, allocated)
+ __field(int, sysctl_rmem)
+ __field(int, rmem_alloc)
+ ),
+
+ TP_fast_assign(
+ strncpy(__entry->name, prot->name, 32);
+ __entry->sysctl_mem = prot->sysctl_mem;
+ __entry->allocated = allocated;
+ __entry->sysctl_rmem = atomic_read(&sk->sk_rmem_alloc);
+ __entry->rmem_alloc = prot->sysctl_rmem[0];
+ ),
+
+ TP_printk("proto:%s sysctl_mem=%ld,%ld,%ld allocated=%ld "
+ "sysctl_rmem=%d rmem_alloc=%d",
+ __entry->name,
+ __entry->sysctl_mem[0],
+ __entry->sysctl_mem[1],
+ __entry->sysctl_mem[2],
+ __entry->allocated,
+ __entry->sysctl_rmem,
+ __entry->rmem_alloc)
+);
+
+#endif /* _TRACE_SOCK_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
diff --git a/net/core/net-traces.c b/net/core/net-traces.c
index 7f1bb2a..b9756f5 100644
--- a/net/core/net-traces.c
+++ b/net/core/net-traces.c
@@ -28,6 +28,7 @@
#include <trace/events/skb.h>
#include <trace/events/net.h>
#include <trace/events/napi.h>
+#include <trace/events/sock.h>
EXPORT_TRACEPOINT_SYMBOL_GPL(kfree_skb);
diff --git a/net/core/sock.c b/net/core/sock.c
index 6e81978..8389032 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -128,6 +128,8 @@
#include <linux/filter.h>
+#include <trace/events/sock.h>
+
#ifdef CONFIG_INET
#include <net/tcp.h>
#endif
@@ -1736,6 +1738,8 @@ suppress_allocation:
return 1;
}
+ trace_sock_exceed_buf_limit(sk, prot, allocated);
+
/* Alas. Undo changes. */
sk->sk_forward_alloc -= amt * SK_MEM_QUANTUM;
atomic_long_sub(amt, prot->memory_allocated);
--
1.7.1
^ permalink raw reply related
* Re: Question about LRO/GRO and TCP acknowledgements
From: Joris van Rantwijk @ 2011-06-14 19:37 UTC (permalink / raw)
To: Ilpo Järvinen; +Cc: Netdev
In-Reply-To: <alpine.DEB.2.00.1106141302110.17529@wel-95.cs.helsinki.fi>
On 2011-06-14, "Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi> wrote:
> BTW, it wouldn't be impossible to create all those "missing" ACKs on
> the TCP layer relatively cheaply when receiving the GRO'ed super
> segment. I'm certainly not opposed you coming up such patch which
> does all that minimal work needed on TCP layer but I think it
> requires also some TSO/GSO related problem solving because TSO/GSO as
> is won't let you create such super ACKs we'd want to send out on that
> single go.
Your super-ACK idea is similar to the solution presented in this paper:
http://www.usenix.org/event/usenix08/tech/full_papers/menon/menon_html/
Actually, I started looking at the GRO code after reading that
paper, hoping to find that Linux has a better way to deal with ACKs.
The super ACK doesn't look easy. It must contain all different ack_seq
values to avoid tripping duplicate ACK detection. Ideally, the ack_seq
values would match real seq values from the received segments.
I don't currently have a setup where I could test these kinds of
changes, so this doesn't seem like a job for me. At least not right now.
Thanks, Joris.
^ permalink raw reply
* Re: [PATCH net-next 0/24] bnx2x: New FW and support for 578xx
From: David Woodhouse @ 2011-06-14 19:23 UTC (permalink / raw)
To: David Miller; +Cc: vladz, mchan, bprakash, eilong, netdev, dmitry, yaniv.rosner
In-Reply-To: <20110614.143349.1778665510231099747.davem@davemloft.net>
On Tue, 14 Jun 2011, David Miller wrote:
> Ok, then let's get this propagated into David's firmware tree.
If there's a git-binary patch in my mailbox that I can apply when I get
home in a few hours, I'll do it immediately. Bwh also has access now, too.
--
dwmw2
^ permalink raw reply
* Re: [E1000-devel] [PATCH] e100: Fix inconsistency in bad frames handling
From: Ben Greear @ 2011-06-14 19:05 UTC (permalink / raw)
To: Brandeburg, Jesse
Cc: Eric Dumazet, Ben Hutchings, Andrea Merello,
e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org
In-Reply-To: <1308077828.4651.5.camel@jbrandeb-mobl2>
On 06/14/2011 11:57 AM, Brandeburg, Jesse wrote:
> On Tue, 2011-06-14 at 10:30 -0700, Ben Greear wrote:
>>>> How would a received skb be flagged as having a CRC error?
>>>>
>>>
>>> maybe some skb->pkt_type = PACKET_INVALID; or something...
>>
>> Jesse: If I can get the ethtool related patches accepted, would
>> you accept patches to e100 (and other Intel drivers) for
>> this feature?
>
> seems like a reasonable thing, but, there is some risk that might
> prevent us turning this on however, because we often like our hardware
> to discard bad frames because (especially) long ones can use quite a few
> buffers.
>
> I still am generally uncomfortable with this idea. We've survived a
> long time without it and it opens up the possibility of extra bugs (like
> possible security issues, etc) with very little opportunity for
> worthwhile gain.
If we make it require root permissions to enable this, and disable
it by default, will that be enough allay your fears?
Also, could print msg to kernel logs when enabling this.
I think that we could skip any intrusive changes (ie, if allowing
rx of long packets is a big problem, just don't allow that for
that particular driver.)
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* [PATCH] gianfar: Use pr_<level>, netdev_<level> and netif_<level>
From: Joe Perches @ 2011-06-14 18:57 UTC (permalink / raw)
To: linux-kernel; +Cc: Sebastian Pöhn, netdev
Use the current logging styles.
Add #define DEBUG to get same output for <foo>_dbg messages.
Convert a few bare printks to pr_err.
Fix a likely copy/paste defect where a test was done with RX values:
if (num_rx_qs > MAX_RX_QS) {
but TX limits were emitted:
printk(KERN_ERR "num_rx_qs(=%d) greater than MAX_RX_QS(=%d)\n",
num_tx_qs, MAX_TX_QS);
Signed-off-by: Joe Perches <joe@perches.com>
---
Sebastian Poehn <sebastian.belden@googlemail.com>
recently submitted a largish patch to gianfar as well.
This may conflict with it.
drivers/net/gianfar.c | 113 +++++++++++++++++-----------------------
drivers/net/gianfar_ethtool.c | 21 ++++----
2 files changed, 58 insertions(+), 76 deletions(-)
diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index 3564551..69cd3d3 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -62,6 +62,9 @@
* The driver then cleans up the buffer.
*/
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#define DEBUG
+
#include <linux/kernel.h>
#include <linux/string.h>
#include <linux/errno.h>
@@ -213,8 +216,7 @@ static int gfar_init_bds(struct net_device *ndev)
} else {
skb = gfar_new_skb(ndev);
if (!skb) {
- pr_err("%s: Can't allocate RX buffers\n",
- ndev->name);
+ netdev_err(ndev, "Can't allocate RX buffers\n");
goto err_rxalloc_fail;
}
rx_queue->rx_skbuff[j] = skb;
@@ -258,9 +260,8 @@ static int gfar_alloc_skb_resources(struct net_device *ndev)
sizeof(struct rxbd8) * priv->total_rx_ring_size,
&addr, GFP_KERNEL);
if (!vaddr) {
- if (netif_msg_ifup(priv))
- pr_err("%s: Could not allocate buffer descriptors!\n",
- ndev->name);
+ netif_err(priv, ifup, ndev,
+ "Could not allocate buffer descriptors!\n");
return -ENOMEM;
}
@@ -290,9 +291,8 @@ static int gfar_alloc_skb_resources(struct net_device *ndev)
tx_queue->tx_skbuff = kmalloc(sizeof(*tx_queue->tx_skbuff) *
tx_queue->tx_ring_size, GFP_KERNEL);
if (!tx_queue->tx_skbuff) {
- if (netif_msg_ifup(priv))
- pr_err("%s: Could not allocate tx_skbuff\n",
- ndev->name);
+ netif_err(priv, ifup, ndev,
+ "Could not allocate tx_skbuff\n");
goto cleanup;
}
@@ -306,9 +306,8 @@ static int gfar_alloc_skb_resources(struct net_device *ndev)
rx_queue->rx_ring_size, GFP_KERNEL);
if (!rx_queue->rx_skbuff) {
- if (netif_msg_ifup(priv))
- pr_err("%s: Could not allocate rx_skbuff\n",
- ndev->name);
+ netif_err(priv, ifup, ndev,
+ "Could not allocate rx_skbuff\n");
goto cleanup;
}
@@ -628,9 +627,9 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev)
num_tx_qs = tx_queues ? *tx_queues : 1;
if (num_tx_qs > MAX_TX_QS) {
- printk(KERN_ERR "num_tx_qs(=%d) greater than MAX_TX_QS(=%d)\n",
- num_tx_qs, MAX_TX_QS);
- printk(KERN_ERR "Cannot do alloc_etherdev, aborting\n");
+ pr_err("num_tx_qs(=%d) greater than MAX_TX_QS(=%d)\n",
+ num_tx_qs, MAX_TX_QS);
+ pr_err("Cannot do alloc_etherdev, aborting\n");
return -EINVAL;
}
@@ -638,9 +637,9 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev)
num_rx_qs = rx_queues ? *rx_queues : 1;
if (num_rx_qs > MAX_RX_QS) {
- printk(KERN_ERR "num_rx_qs(=%d) greater than MAX_RX_QS(=%d)\n",
- num_tx_qs, MAX_TX_QS);
- printk(KERN_ERR "Cannot do alloc_etherdev, aborting\n");
+ pr_err("num_rx_qs(=%d) greater than MAX_RX_QS(=%d)\n",
+ num_rx_qs, MAX_RX_QS);
+ pr_err("Cannot do alloc_etherdev, aborting\n");
return -EINVAL;
}
@@ -1163,8 +1162,7 @@ static int gfar_probe(struct platform_device *ofdev)
err = register_netdev(dev);
if (err) {
- printk(KERN_ERR "%s: Cannot register net device, aborting.\n",
- dev->name);
+ pr_err("%s: Cannot register net device, aborting\n", dev->name);
goto register_fail;
}
@@ -1215,17 +1213,17 @@ static int gfar_probe(struct platform_device *ofdev)
gfar_init_sysfs(dev);
/* Print out the device info */
- printk(KERN_INFO DEVICE_NAME "%pM\n", dev->name, dev->dev_addr);
+ netdev_info(dev, "mac: %pM\n", dev->dev_addr);
/* Even more device info helps when determining which kernel */
/* provided which set of benchmarks. */
- printk(KERN_INFO "%s: Running with NAPI enabled\n", dev->name);
+ netdev_info(dev, "Running with NAPI enabled\n");
for (i = 0; i < priv->num_rx_queues; i++)
- printk(KERN_INFO "%s: RX BD ring size for Q[%d]: %d\n",
- dev->name, i, priv->rx_queue[i]->rx_ring_size);
+ netdev_info(dev, "RX BD ring size for Q[%d]: %d\n",
+ i, priv->rx_queue[i]->rx_ring_size);
for(i = 0; i < priv->num_tx_queues; i++)
- printk(KERN_INFO "%s: TX BD ring size for Q[%d]: %d\n",
- dev->name, i, priv->tx_queue[i]->tx_ring_size);
+ netdev_info(dev, "TX BD ring size for Q[%d]: %d\n",
+ i, priv->tx_queue[i]->tx_ring_size);
return 0;
@@ -1858,34 +1856,30 @@ static int register_grp_irqs(struct gfar_priv_grp *grp)
* Transmit, and Receive */
if ((err = request_irq(grp->interruptError, gfar_error, 0,
grp->int_name_er,grp)) < 0) {
- if (netif_msg_intr(priv))
- printk(KERN_ERR "%s: Can't get IRQ %d\n",
- dev->name, grp->interruptError);
+ netif_err(priv, intr, dev, "Can't get IRQ %d\n",
+ grp->interruptError);
goto err_irq_fail;
}
if ((err = request_irq(grp->interruptTransmit, gfar_transmit,
0, grp->int_name_tx, grp)) < 0) {
- if (netif_msg_intr(priv))
- printk(KERN_ERR "%s: Can't get IRQ %d\n",
- dev->name, grp->interruptTransmit);
+ netif_err(priv, intr, dev, "Can't get IRQ %d\n",
+ grp->interruptTransmit);
goto tx_irq_fail;
}
if ((err = request_irq(grp->interruptReceive, gfar_receive, 0,
grp->int_name_rx, grp)) < 0) {
- if (netif_msg_intr(priv))
- printk(KERN_ERR "%s: Can't get IRQ %d\n",
- dev->name, grp->interruptReceive);
+ netif_err(priv, intr, dev, "Can't get IRQ %d\n",
+ grp->interruptReceive);
goto rx_irq_fail;
}
} else {
if ((err = request_irq(grp->interruptTransmit, gfar_interrupt, 0,
grp->int_name_tx, grp)) < 0) {
- if (netif_msg_intr(priv))
- printk(KERN_ERR "%s: Can't get IRQ %d\n",
- dev->name, grp->interruptTransmit);
+ netif_err(priv, intr, dev, "Can't get IRQ %d\n",
+ grp->interruptTransmit);
goto err_irq_fail;
}
}
@@ -2354,9 +2348,7 @@ static int gfar_change_mtu(struct net_device *dev, int new_mtu)
frame_size += VLAN_HLEN;
if ((frame_size < 64) || (frame_size > JUMBO_FRAME_SIZE)) {
- if (netif_msg_drv(priv))
- printk(KERN_ERR "%s: Invalid MTU setting\n",
- dev->name);
+ netif_err(priv, drv, dev, "Invalid MTU setting\n");
return -EINVAL;
}
@@ -2776,9 +2768,7 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)
gfar_process_frame(dev, skb, amount_pull);
} else {
- if (netif_msg_rx_err(priv))
- printk(KERN_WARNING
- "%s: Missing skb!\n", dev->name);
+ netif_warn(priv, rx_err, dev, "Missing skb!\n");
rx_queue->stats.rx_dropped++;
priv->extra_stats.rx_skbmissing++;
}
@@ -2981,10 +2971,9 @@ static void adjust_link(struct net_device *dev)
ecntrl &= ~(ECNTRL_R100);
break;
default:
- if (netif_msg_link(priv))
- printk(KERN_WARNING
- "%s: Ack! Speed (%d) is not 10/100/1000!\n",
- dev->name, phydev->speed);
+ netif_warn(priv, link, dev,
+ "Ack! Speed (%d) is not 10/100/1000!\n",
+ phydev->speed);
break;
}
@@ -3189,8 +3178,8 @@ static irqreturn_t gfar_error(int irq, void *grp_id)
/* Hmm... */
if (netif_msg_rx_err(priv) || netif_msg_tx_err(priv))
- printk(KERN_DEBUG "%s: error interrupt (ievent=0x%08x imask=0x%08x)\n",
- dev->name, events, gfar_read(®s->imask));
+ netdev_dbg(dev, "error interrupt (ievent=0x%08x imask=0x%08x)\n",
+ events, gfar_read(®s->imask));
/* Update the error counters */
if (events & IEVENT_TXE) {
@@ -3203,9 +3192,8 @@ static irqreturn_t gfar_error(int irq, void *grp_id)
if (events & IEVENT_XFUN) {
unsigned long flags;
- if (netif_msg_tx_err(priv))
- printk(KERN_DEBUG "%s: TX FIFO underrun, "
- "packet dropped.\n", dev->name);
+ netif_dbg(priv, tx_err, dev,
+ "TX FIFO underrun, packet dropped\n");
dev->stats.tx_dropped++;
priv->extra_stats.tx_underrun++;
@@ -3218,8 +3206,7 @@ static irqreturn_t gfar_error(int irq, void *grp_id)
unlock_tx_qs(priv);
local_irq_restore(flags);
}
- if (netif_msg_tx_err(priv))
- printk(KERN_DEBUG "%s: Transmit Error\n", dev->name);
+ netif_dbg(priv, tx_err, dev, "Transmit Error\n");
}
if (events & IEVENT_BSY) {
dev->stats.rx_errors++;
@@ -3227,29 +3214,25 @@ static irqreturn_t gfar_error(int irq, void *grp_id)
gfar_receive(irq, grp_id);
- if (netif_msg_rx_err(priv))
- printk(KERN_DEBUG "%s: busy error (rstat: %x)\n",
- dev->name, gfar_read(®s->rstat));
+ netif_dbg(priv, rx_err, dev, "busy error (rstat: %x)\n",
+ gfar_read(®s->rstat));
}
if (events & IEVENT_BABR) {
dev->stats.rx_errors++;
priv->extra_stats.rx_babr++;
- if (netif_msg_rx_err(priv))
- printk(KERN_DEBUG "%s: babbling RX error\n", dev->name);
+ netif_dbg(priv, rx_err, dev, "babbling RX error\n");
}
if (events & IEVENT_EBERR) {
priv->extra_stats.eberr++;
- if (netif_msg_rx_err(priv))
- printk(KERN_DEBUG "%s: bus error\n", dev->name);
+ netif_dbg(priv, rx_err, dev, "bus error\n");
}
- if ((events & IEVENT_RXC) && netif_msg_rx_status(priv))
- printk(KERN_DEBUG "%s: control frame\n", dev->name);
+ if (events & IEVENT_RXC)
+ netif_dbg(priv, rx_status, dev, "control frame\n");
if (events & IEVENT_BABT) {
priv->extra_stats.tx_babt++;
- if (netif_msg_tx_err(priv))
- printk(KERN_DEBUG "%s: babbling TX error\n", dev->name);
+ netif_dbg(priv, tx_err, dev, "babbling TX error\n");
}
return IRQ_HANDLED;
}
diff --git a/drivers/net/gianfar_ethtool.c b/drivers/net/gianfar_ethtool.c
index 493d743..92d7ac0 100644
--- a/drivers/net/gianfar_ethtool.c
+++ b/drivers/net/gianfar_ethtool.c
@@ -16,6 +16,8 @@
* by reference.
*/
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
#include <linux/kernel.h>
#include <linux/string.h>
#include <linux/errno.h>
@@ -375,13 +377,13 @@ static int gfar_scoalesce(struct net_device *dev, struct ethtool_coalesce *cvals
/* Check the bounds of the values */
if (cvals->rx_coalesce_usecs > GFAR_MAX_COAL_USECS) {
pr_info("Coalescing is limited to %d microseconds\n",
- GFAR_MAX_COAL_USECS);
+ GFAR_MAX_COAL_USECS);
return -EINVAL;
}
if (cvals->rx_max_coalesced_frames > GFAR_MAX_COAL_FRAMES) {
pr_info("Coalescing is limited to %d frames\n",
- GFAR_MAX_COAL_FRAMES);
+ GFAR_MAX_COAL_FRAMES);
return -EINVAL;
}
@@ -404,13 +406,13 @@ static int gfar_scoalesce(struct net_device *dev, struct ethtool_coalesce *cvals
/* Check the bounds of the values */
if (cvals->tx_coalesce_usecs > GFAR_MAX_COAL_USECS) {
pr_info("Coalescing is limited to %d microseconds\n",
- GFAR_MAX_COAL_USECS);
+ GFAR_MAX_COAL_USECS);
return -EINVAL;
}
if (cvals->tx_max_coalesced_frames > GFAR_MAX_COAL_FRAMES) {
pr_info("Coalescing is limited to %d frames\n",
- GFAR_MAX_COAL_FRAMES);
+ GFAR_MAX_COAL_FRAMES);
return -EINVAL;
}
@@ -464,8 +466,7 @@ static int gfar_sringparam(struct net_device *dev, struct ethtool_ringparam *rva
return -EINVAL;
if (!is_power_of_2(rvals->rx_pending)) {
- printk("%s: Ring sizes must be a power of 2\n",
- dev->name);
+ netdev_err(dev, "Ring sizes must be a power of 2\n");
return -EINVAL;
}
@@ -473,8 +474,7 @@ static int gfar_sringparam(struct net_device *dev, struct ethtool_ringparam *rva
return -EINVAL;
if (!is_power_of_2(rvals->tx_pending)) {
- printk("%s: Ring sizes must be a power of 2\n",
- dev->name);
+ netdev_err(dev, "Ring sizes must be a power of 2\n");
return -EINVAL;
}
@@ -700,7 +700,7 @@ static int gfar_ethflow_to_filer_table(struct gfar_private *priv, u64 ethflow, u
cmp_rqfpr = RQFPR_IPV6 |RQFPR_UDP;
break;
default:
- printk(KERN_ERR "Right now this class is not supported\n");
+ pr_err("Right now this class is not supported\n");
return 0;
}
@@ -715,8 +715,7 @@ static int gfar_ethflow_to_filer_table(struct gfar_private *priv, u64 ethflow, u
}
if (i == MAX_FILER_IDX + 1) {
- printk(KERN_ERR "No parse rule found, ");
- printk(KERN_ERR "can't create hash rules\n");
+ pr_err("No parse rule found, can't create hash rules\n");
return 0;
}
--
1.7.6.rc0
^ permalink raw reply related
* Re: [E1000-devel] [PATCH] e100: Fix inconsistency in bad frames handling
From: Brandeburg, Jesse @ 2011-06-14 18:57 UTC (permalink / raw)
To: Ben Greear
Cc: Eric Dumazet, Ben Hutchings, Andrea Merello,
e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org
In-Reply-To: <4DF79AB1.2080109@candelatech.com>
On Tue, 2011-06-14 at 10:30 -0700, Ben Greear wrote:
> >> How would a received skb be flagged as having a CRC error?
> >>
> >
> > maybe some skb->pkt_type = PACKET_INVALID; or something...
>
> Jesse: If I can get the ethtool related patches accepted, would
> you accept patches to e100 (and other Intel drivers) for
> this feature?
seems like a reasonable thing, but, there is some risk that might
prevent us turning this on however, because we often like our hardware
to discard bad frames because (especially) long ones can use quite a few
buffers.
I still am generally uncomfortable with this idea. We've survived a
long time without it and it opens up the possibility of extra bugs (like
possible security issues, etc) with very little opportunity for
worthwhile gain.
Jesse
^ permalink raw reply
* Re: [PATCH net-next 0/24] bnx2x: New FW and support for 578xx
From: David Woodhouse @ 2011-06-14 18:52 UTC (permalink / raw)
To: David Miller; +Cc: mchan, vladz, bprakash, eilong, netdev, dmitry, yaniv.rosner
In-Reply-To: <20110614.134653.607599293786464713.davem@davemloft.net>
On Tue, 14 Jun 2011, David Miller wrote:
> > When David Woodhouse integrates the firmware and a new firmware rpm is
> > made available for users, then the new driver will work.
Tarballs are made daily, as soon as the firmware is submitted (which
doesn't have to wait until the driver is patched). There is no issue with
bisecting, because any ABI change in the firmware means a *new* firmware
file which can be instslled in *parallel*.
> This is terrible.
Nonsense. It just means you want to be using an up-to-date copy of the
firmware, which is hardly a problem.
--
dwmw2
^ permalink raw reply
* Re: [RFC 2/2] ethtool: Add support for DMA Coalescing feature config to ethtool.
From: Ben Hutchings @ 2011-06-14 18:51 UTC (permalink / raw)
To: Carolyn Wyborny; +Cc: netdev
In-Reply-To: <1308071606-6333-1-git-send-email-carolyn.wyborny@intel.com>
Carolyn Wyborny wrote:
> This RFC patch adds functions to get and set DMA Coalescing feature
> configuration.
>
> Signed-off-by: Carolyn Wyborny <carolyn.wyborny@intel.com>
> ---
> include/linux/ethtool.h | 15 ++++++++++++++-
> net/core/ethtool.c | 32 ++++++++++++++++++++++++++++++++
> 2 files changed, 46 insertions(+), 1 deletions(-)
>
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index c6a850a..efed754 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -703,6 +703,14 @@ enum ethtool_sfeatures_retval_bits {
> ETHTOOL_F_COMPAT__BIT,
> };
>
> +/* for configuring DMA coalescing parameters of chip */
> +struct ethtool_dmac {
> + __u32 cmd; /* ETHTOOL_{G,S}DMAC */
> +
> + /* tune setting, options and validation determined by device. */
> + __u32 tune;
> +};
[...]
I don't think we should be adding operations that have no generic
semantics at all. Further, we will not add any operations without at
least one implementation as an example.
Secondly, ethtool operations that only get/set a 32-bit value should
use struct ethtool_value for the parameter.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH net-next 0/24] bnx2x: New FW and support for 578xx
From: David Miller @ 2011-06-14 18:33 UTC (permalink / raw)
To: vladz; +Cc: mchan, bprakash, eilong, netdev, dmitry, yaniv.rosner, dwmw2
In-Reply-To: <35D366A0-235E-44ED-8D5B-AEDB1D8AC0F5@broadcom.com>
From: "Vladislav Zolotarov" <vladz@broadcom.com>
Date: Tue, 14 Jun 2011 11:32:04 -0700
> This requires some sync between Dave and David: David should always
> be the first to integrate a new FW and only then Dave should
> inegrate a driver patch series with the support for this new FW. The
> responsibility for the ordering should be on the driver owner: in
> our case, i shouldn't be sending a net-next patch series until the
> linux-firmware patch is applied.
Ok, then let's get this propagated into David's firmware tree.
^ permalink raw reply
* RE: [PATCH net-next 2/2] drivers/net: Remove casts of void *
From: Ajit.Khaparde @ 2011-06-14 18:31 UTC (permalink / raw)
To: joe, netdev
Cc: acme, jcliburn, chris.snook, jie.yang, Sathya.Perla,
subbu.seetharaman, rmody, ddutt, sjur.brandeland, divy, leitao,
amit.salecha, ron.mercer, linux-driver, anirban.chakraborty,
jdmason, linux-net-drivers, shodgson, bhutchings, venza, dave,
linux-kernel
In-Reply-To: <C4E199C967B4824F9C9DF4C42CE4A3B8591B7F0D78@EXMAIL.ad.emulex.com>
> -----Original Message-----
> From: Joe Perches [mailto:joe@perches.com]
> Subject: [PATCH net-next 2/2] drivers/net: Remove casts of void *
>
> Unnecessary casts of void * clutter the code.
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> diff --git a/drivers/net/benet/be_cmds.c b/drivers/net/benet/be_cmds.c
> index 0c12c2d..30719f5 100644
> --- a/drivers/net/benet/be_cmds.c
> +++ b/drivers/net/benet/be_cmds.c
> @@ -2334,8 +2334,7 @@ int be_cmd_get_cntl_attributes(struct be_adapter
> *adapter)
>
> status = be_mbox_notify_wait(adapter);
> if (!status) {
> - attribs = (struct mgmt_controller_attrib *)( attribs_cmd.va
> +
> - sizeof(struct be_cmd_resp_hdr));
> + attribs = attribs_cmd.va + sizeof(struct be_cmd_resp_hdr);
> adapter->hba_port_num = attribs->hba_attribs.phy_port;
> }
>
> diff --git a/drivers/net/benet/be_ethtool.c
> b/drivers/net/benet/be_ethtool.c
> index facfe3c..5572c78 100644
> --- a/drivers/net/benet/be_ethtool.c
> +++ b/drivers/net/benet/be_ethtool.c
> @@ -386,7 +386,7 @@ static int be_get_settings(struct net_device
> *netdev, struct ethtool_cmd *ecmd)
> }
> status = be_cmd_get_phy_info(adapter, &phy_cmd);
> if (!status) {
> - resp = (struct be_cmd_resp_get_phy_info *)
> phy_cmd.va;
> + resp = phy_cmd.va;
> intf_type = le16_to_cpu(resp->interface_type);
>
> switch (intf_type) {
> @@ -690,7 +690,7 @@ be_read_eeprom(struct net_device *netdev, struct
> ethtool_eeprom *eeprom,
> status = be_cmd_get_seeprom_data(adapter, &eeprom_cmd);
>
> if (!status) {
> - resp = (struct be_cmd_resp_seeprom_read *) eeprom_cmd.va;
> + resp = eeprom_cmd.va;
> memcpy(data, resp->seeprom_data + eeprom->offset, eeprom-
> >len);
> }
> dma_free_coherent(&adapter->pdev->dev, eeprom_cmd.size,
> eeprom_cmd.va,
The be2net part of the patch looks good.
Acked-by: Ajit Khaparde ajit.khaparde@emulex.com
^ permalink raw reply
* Re: [PATCH net-next 0/24] bnx2x: New FW and support for 578xx
From: Vladislav Zolotarov @ 2011-06-14 18:32 UTC (permalink / raw)
To: David Miller
Cc: Michael Chan, Bhanu (Venkata Bhanu Prakash) Gollapudi,
Eilon Greenstein, netdev@vger.kernel.org, Dmitry Kravkov,
Yaniv Rosner, dwmw2@infradead.org
In-Reply-To: <20110614.134653.607599293786464713.davem@davemloft.net>
On 14 ביונ 2011, at 20:47, "David Miller" <davem@davemloft.net> wrote:
> From: "Michael Chan" <mchan@broadcom.com>
> Date: Tue, 14 Jun 2011 10:35:53 -0700
>
>> If the firmware is not integrated into net-next-2.6, the newly patched
>> driver won't work. You get an error at run time saying the firmware is
>> not available.
>>
>> When David Woodhouse integrates the firmware and a new firmware rpm is
>> made available for users, then the new driver will work.
>
> This is terrible.
>
Another option is that the person that wills to run a bisect has to start with pulling the latest linux-firmware git, copying its contents into the /lib/firmware and then start a bisect.
This requires some sync between Dave and David: David should always be the first to integrate a new FW and only then Dave should inegrate a driver patch series with the support for this new FW. The responsibility for the ordering should be on the driver owner: in our case, i shouldn't be sending a net-next patch series until the linux-firmware patch is applied.
It doesn't sound that terrible price for having a kernel tree clean from binaries blobs... ;)
Vlad
^ permalink raw reply
* RE: [PATCH net-next 2/2] drivers/net: Remove casts of void *
From: Sjur BRENDELAND @ 2011-06-14 17:53 UTC (permalink / raw)
To: Joe Perches, netdev@vger.kernel.org
Cc: Arnaldo Carvalho de Melo, Jay Cliburn, Chris Snook, Jie Yang,
Sathya Perla, Subbu Seetharaman, Ajit Khaparde, Rasesh Mody,
Debashis Dutt, Divy Le Ray, Breno Leitao, Amit Kumar Salecha,
Ron Mercer, linux-driver@qlogic.com, Anirban Chakraborty,
Jon Mason, Solarflare linux maintainers, Steve Hodgson,
Ben Hutchings, Daniele Venzano, David Dillow, "li
In-Reply-To: <486a21b708a9dce0c1c0059a21a2f9b66dac9e3c.1308024069.git.joe@perches.com>
Hi,
[Joe Perches]:
> Unnecessary casts of void * clutter the code.
>--- a/drivers/net/caif/caif_shmcore.c
>+++ b/drivers/net/caif/caif_shmcore.c
>@@ -134,7 +134,7 @@ int caif_shmdrv_rx_cb(u32 mbx_msg, void *priv)
> u32 avail_emptybuff = 0;
> unsigned long flags = 0;
>
>- pshm_drv = (struct shmdrv_layer *)priv;
>+ pshm_drv = priv;
>
CAIF parts looks good to me,
Acked-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
^ permalink raw reply
* Re: [PATCH net-next 0/24] bnx2x: New FW and support for 578xx
From: David Miller @ 2011-06-14 17:46 UTC (permalink / raw)
To: mchan; +Cc: vladz, bprakash, eilong, netdev, dmitry, yaniv.rosner, dwmw2
In-Reply-To: <1308072953.9492.4.camel@HP1>
From: "Michael Chan" <mchan@broadcom.com>
Date: Tue, 14 Jun 2011 10:35:53 -0700
> If the firmware is not integrated into net-next-2.6, the newly patched
> driver won't work. You get an error at run time saying the firmware is
> not available.
>
> When David Woodhouse integrates the firmware and a new firmware rpm is
> made available for users, then the new driver will work.
This is terrible.
^ permalink raw reply
* Re: [ath9k-devel] [PATCH net-next 1/2] wireless: Remove casts of void *
From: Christian Lamparter @ 2011-06-14 17:43 UTC (permalink / raw)
To: Joe Perches
Cc: Pavel Roskin, netdev-u79uwXL29TY76Z2rM5mHXA, Intel Linux Wireless,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA, John W. Linville,
Jouni Malinen, Senthil Balasubramanian,
ath9k-devel-xDcbHBWguxHbcTqmT+pZeQ, Wey-Yi Guy,
Vasanthakumar Thiagarajan, Stanislaw Gruszka
In-Reply-To: <1308069831.21026.9.camel@Joe-Laptop>
On Tuesday 14 June 2011 18:43:51 Joe Perches wrote:
> On Tue, 2011-06-14 at 12:31 -0400, Pavel Roskin wrote:
> > On 06/14/2011 12:02 AM, Joe Perches wrote:
> > > /* Get the message ID */
> > > - msg_id = (__be16 *) ((void *) htc_hdr +
> > > - sizeof(struct htc_frame_hdr));
> > > + msg_id = (void *)htc_hdr + sizeof(struct htc_frame_hdr);
> > I would never do stuff like this without verifying by sparse that no
> > warnings are introduced.
>
> I did that. I believe there are no new warnings.
>
> > Sparse warnings should be avoided to keep sparse checks useful.
> > Otherwise, important warnings would drown in the noise.
>
> $ make allyesconfig
> $ git log -1 --pretty=oneline drivers/net/wireless/ath/ath9k/htc_hst.c
> 337c22b774ff7f007b90b266b25c9a33ff555c48 wireless: Remove casts of void *
> $ make C=2 drivers/net/wireless/ath/ath9k/htc_hst.o
Just a quick FYI: To perform endianness checks, you may define __CHECK_ENDIAN__:
make C=2 CF="-D__CHECK_ENDIAN__"
[Although, it doesn't look like your patch introduced any new endianness
problems]
Regards,
Chr
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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: [PATCH net-next 0/24] bnx2x: New FW and support for 578xx
From: Michael Chan @ 2011-06-14 17:35 UTC (permalink / raw)
To: David Miller
Cc: Vladislav Zolotarov, Bhanu (Venkata Bhanu Prakash) Gollapudi,
Eilon Greenstein, netdev@vger.kernel.org, Dmitry Kravkov,
Yaniv Rosner, dwmw2@infradead.org
In-Reply-To: <20110614.133200.1595212089561436266.davem@davemloft.net>
On Tue, 2011-06-14 at 10:32 -0700, David Miller wrote:
> From: "Vlad Zolotarov" <vladz@broadcom.com>
> Date: Tue, 14 Jun 2011 19:22:40 +0300
>
> > On Tuesday 14 June 2011 18:54:02 David Miller wrote:
> >> From: "Vlad Zolotarov" <vladz@broadcom.com>
> >> Date: Tue, 14 Jun 2011 14:32:31 +0300
> >>
> >> > We also send the new FW files to David Woodhouse. It's the first time we
> >> > submit to the linux-firmware git so I hope we don't mess the things up:
> >> > we DO NOT patch firmware/WHENCE and firmware/Makefile files in our
> >> > net-next patches and we patch the WHENCE file in the linux-firmware
> >> > patch. Dave, pls., confirm if it's a correct procedure.
> >> >
> >> > If it is, pls., note that the bnx2x driver will compile but won't work
> >> > until u integrate the new FW into the kernel tree.
> >>
> >> David Woodhouse, what do you think we should do here?
> >>
> >> I'm inclined to integrate the firmware into net-next-2.6 since this
> >> is the only way to keep the driver working consistently.
> >>
> >> Otherwise people won't be able to git bisect properly, and that sucks.
> >
> > Dave, as long as /lib/firmware contains all existing firmwares there should be
> > no problems, isn't it?
>
> You're saying above that the driver will compile but not work in net-next-2.6
> until the FW is integrated, that's not acceptable, the tree must be fully
> bisectable for people trying to track down bugs.
>
> Which is it?
>
If the firmware is not integrated into net-next-2.6, the newly patched
driver won't work. You get an error at run time saying the firmware is
not available.
When David Woodhouse integrates the firmware and a new firmware rpm is
made available for users, then the new driver will work.
^ permalink raw reply
* Re: [PATCH] net: Ensure tx watchdog failures always print.
From: Ben Greear @ 2011-06-14 17:41 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20110614.133329.165803790485169225.davem@davemloft.net>
On 06/14/2011 10:33 AM, David Miller wrote:
> From: greearb@candelatech.com
> Date: Tue, 14 Jun 2011 09:50:13 -0700
>
>> From: Ben Greear<greearb@candelatech.com>
>>
>> These are too important to not have some log message
>> generated. But, keep the logic that only prints
>> a single stack dump.
>>
>> Signed-off-by: Ben Greear<greearb@candelatech.com>
>
> We specifically made this print out only once, and yes
> all of it.
>
> Because it spams people's logs to the point that the message
> is not useful and only scrolls out other more important
> messages.
>
> I'm not applying this, sorry.
Is it really legit for a driver to spam this message? I hit
this due to bugs in my code that sent pkts to the wrong
driver queue, but aside from that, we rarely if ever see
the message print.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [PATCH] net: Ensure tx watchdog failures always print.
From: David Miller @ 2011-06-14 17:34 UTC (permalink / raw)
To: greearb; +Cc: eric.dumazet, netdev
In-Reply-To: <4DF79979.2070309@candelatech.com>
From: Ben Greear <greearb@candelatech.com>
Date: Tue, 14 Jun 2011 10:25:13 -0700
> Will re-send.
Please, don't bother, see my other reply.
^ permalink raw reply
* Re: [PATCH] net: Ensure tx watchdog failures always print.
From: David Miller @ 2011-06-14 17:33 UTC (permalink / raw)
To: greearb; +Cc: netdev
In-Reply-To: <1308070213-13244-1-git-send-email-greearb@candelatech.com>
From: greearb@candelatech.com
Date: Tue, 14 Jun 2011 09:50:13 -0700
> From: Ben Greear <greearb@candelatech.com>
>
> These are too important to not have some log message
> generated. But, keep the logic that only prints
> a single stack dump.
>
> Signed-off-by: Ben Greear <greearb@candelatech.com>
We specifically made this print out only once, and yes
all of it.
Because it spams people's logs to the point that the message
is not useful and only scrolls out other more important
messages.
I'm not applying this, sorry.
^ permalink raw reply
* Re: [PATCH net-next 0/24] bnx2x: New FW and support for 578xx
From: David Miller @ 2011-06-14 17:32 UTC (permalink / raw)
To: vladz; +Cc: mchan, bprakash, eilong, netdev, dmitry, yaniv.rosner, dwmw2
In-Reply-To: <201106141922.40906.vladz@broadcom.com>
From: "Vlad Zolotarov" <vladz@broadcom.com>
Date: Tue, 14 Jun 2011 19:22:40 +0300
> On Tuesday 14 June 2011 18:54:02 David Miller wrote:
>> From: "Vlad Zolotarov" <vladz@broadcom.com>
>> Date: Tue, 14 Jun 2011 14:32:31 +0300
>>
>> > We also send the new FW files to David Woodhouse. It's the first time we
>> > submit to the linux-firmware git so I hope we don't mess the things up:
>> > we DO NOT patch firmware/WHENCE and firmware/Makefile files in our
>> > net-next patches and we patch the WHENCE file in the linux-firmware
>> > patch. Dave, pls., confirm if it's a correct procedure.
>> >
>> > If it is, pls., note that the bnx2x driver will compile but won't work
>> > until u integrate the new FW into the kernel tree.
>>
>> David Woodhouse, what do you think we should do here?
>>
>> I'm inclined to integrate the firmware into net-next-2.6 since this
>> is the only way to keep the driver working consistently.
>>
>> Otherwise people won't be able to git bisect properly, and that sucks.
>
> Dave, as long as /lib/firmware contains all existing firmwares there should be
> no problems, isn't it?
You're saying above that the driver will compile but not work in net-next-2.6
until the FW is integrated, that's not acceptable, the tree must be fully
bisectable for people trying to track down bugs.
Which is it?
^ permalink raw reply
* Re: [PATCH] e100: Fix inconsistency in bad frames handling
From: Ben Greear @ 2011-06-14 17:30 UTC (permalink / raw)
To: Eric Dumazet
Cc: Ben Hutchings, Andrea Merello, netdev, Brandeburg, Jesse,
e1000-devel@lists.sourceforge.net
In-Reply-To: <1307392197.2642.14.camel@edumazet-laptop>
On 06/06/2011 01:29 PM, Eric Dumazet wrote:
> Le lundi 06 juin 2011 à 21:15 +0100, Ben Hutchings a écrit :
>> On Mon, 2011-06-06 at 10:56 -0700, Ben Greear wrote:
>>> On 06/06/2011 10:49 AM, Brandeburg, Jesse wrote:
>>>>
>>>> <added netdev>, removed other useless lists.
>>>>
>>>> On Sat, 4 Jun 2011, Andrea Merello wrote:
>>>>> In e100 driver it seems that the intention was to accept bad frames in
>>>>> promiscuous mode and loopback mode.
>>>>> I think this is evident because of the following code in the driver:
>>>>>
>>>>> if (nic->flags& promiscuous || nic->loopback) {
>>>>> config->rx_save_bad_frames = 0x1; /* 1=save, 0=discard */
>>>>> config->rx_discard_short_frames = 0x0; /* 1=discard, 0=save */
>>>>> config->promiscuous_mode = 0x1; /* 1=on, 0=off */
>>>>> }
>>>>>
>>>>
>>>> Hi, thanks for your work on e100.
>>>>
>>>>> However this intention is not really realized because bad frames are
>>>>> discarded later by SW check.
>>>>> This patch finally honors the above intention, making the RX code to
>>>>> let bad frames to pass when the NIC is in promiscuous or loopback
>>>>> mode.
>>>>
>>>> I think this may be a mistake by the authors of the software developers
>>>> manual. The manual suggests that save bad frames and save short frames
>>>> should be enabled in promisc mode, but all of our other drivers *do not*
>>>> save bad frames when in promiscuous mode (by design). This is intentional
>>>> because a bad frame is just that, bad, and with no hope of knowing if the
>>>> data in it is okay/malicious/other. I understand your reasoning above,
>>>> but realistically the rx_save_bad_frames should NOT be set. I'd ack a
>>>> patch to comment that line out.
>>>>
>>>>> This helped me a lot to debug an FPGA ethernet core.
>>>>> Maybe it can be also useful to someone else..
>>>>
>>>> I think this patch is just that, debug only. As a developer I understand
>>>> why this is useful, but there is no reason any normal user would be able
>>>> to benefit from this, so for now, sorry:
>>>>
>>>> NACK.
>>>
>>> I think anyone sniffing a funky network would have benefit in
>>> receiving all frames. So, while it shouldn't be enabled by default,
>>> it would be nice to have an ethtool command to turn on receiving
>>> bad-crc frames, as well as receiving the 4-byte CRC on the end of
>>> the packets.
>>>
>>> It just so happens I have such a patch, in case others agree :)
>>
>> How would a received skb be flagged as having a CRC error?
>>
>
> maybe some skb->pkt_type = PACKET_INVALID; or something...
Jesse: If I can get the ethtool related patches accepted, would
you accept patches to e100 (and other Intel drivers) for
this feature?
Thanks,
Ben
>
>
>
> --
> 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
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
------------------------------------------------------------------------------
EditLive Enterprise is the world's most technically advanced content
authoring tool. Experience the power of Track Changes, Inline Image
Editing and ensure content is compliant with Accessibility Checking.
http://p.sf.net/sfu/ephox-dev2dev
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply
* Re: [PATCH] net: Ensure tx watchdog failures always print.
From: Ben Greear @ 2011-06-14 17:25 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1308071667.4870.17.camel@edumazet-laptop>
On 06/14/2011 10:14 AM, Eric Dumazet wrote:
> Le mardi 14 juin 2011 à 09:50 -0700, greearb@candelatech.com a écrit :
>> From: Ben Greear<greearb@candelatech.com>
>>
>> These are too important to not have some log message
>> generated. But, keep the logic that only prints
>> a single stack dump.
>>
>> Signed-off-by: Ben Greear<greearb@candelatech.com>
>> ---
>> :100644 100644 90939ba... fce5558... M net/sched/sch_generic.c
>> net/sched/sch_generic.c | 16 ++++++++++++++--
>> 1 files changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
>> index 90939ba..fce5558 100644
>> --- a/net/sched/sch_generic.c
>> +++ b/net/sched/sch_generic.c
>> @@ -252,8 +252,20 @@ static void dev_watchdog(unsigned long arg)
>>
>> if (some_queue_timedout) {
>> char drivername[64];
>> - WARN_ONCE(1, KERN_INFO "NETDEV WATCHDOG: %s (%s): transmit queue %u timed out\n",
>> - dev->name, netdev_drivername(dev, drivername, 64), i);
>> + static bool do_once = true;
>> + printk(KERN_INFO "NETDEV WATCHDOG: %s (%s):"
>> + " transmit queue %u timed out,"
>> + " trans_start: %lu, wd-timeout: %i"
>> + " jiffies: %lu tx-queues: %i\n",
>> + dev->name,
>> + netdev_drivername(dev, drivername, 64),
>> + i,
>> + trans_start, dev->watchdog_timeo,
>> + jiffies, dev->num_tx_queues);
>> + if (do_once) {
>> + do_once = false;
>> + WARN_ON(1);
>
> You could use WARN_ON_ONCE(1) here
Er, right..don't know what I was thinking.
Will re-send.
Thanks,
Ben
>
>> + }
>> dev->netdev_ops->ndo_tx_timeout(dev);
>> }
>> if (!mod_timer(&dev->watchdog_timer,
>
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: IGMP snooping: set mrouters_only flag for IPv4 traffic properly
From: Stephen Hemminger @ 2011-06-14 17:22 UTC (permalink / raw)
To: Fernando Luis Vazquez Cao; +Cc: Herbert Xu, netdev, Hayato Kakuta
In-Reply-To: <1307934163.3550.2.camel@nausicaa>
On Mon, 13 Jun 2011 12:02:43 +0900
Fernando Luis Vazquez Cao <fernando@oss.ntt.co.jp> wrote:
> Upon reception of a IGMP/IGMPv2 membership report the kernel sets the
> mrouters_only flag in a skb that may be a clone of the original skb, which
> means that sometimes the bridge loses track of membership report packets (cb
> buffers are tied to a specifici skb and not shared) and it ends up forwading
> join requests to the bridge interface.
>
> This can cause unexpected membership timeouts and intermitent/permanent loss of connectivity as described in RFC 4541 [2.1.1. IGMP Forwarding Rules]:
>
> A snooping switch should forward IGMP Membership Reports only to
> those ports where multicast routers are attached.
> [...]
> Sending membership reports to other hosts can result, for IGMPv1
> and IGMPv2, in unintentionally preventing a host from joining a
> specific multicast group.
>
>
> Signed-off-by: Fernando Luis Vazquez Cao <fernando@oss.ntt.co.jp>
> Tested-by: Hayato Kakuta <kakuta.hayato@oss.ntt.co.jp>
> ---
>
> diff -urNp linux-3.0-rc2-orig/net/bridge/br_multicast.c linux-3.0-rc2/net/bridge/br_multicast.c
> --- linux-3.0-rc2-orig/net/bridge/br_multicast.c 2011-06-09 13:34:04.164261031 +0900
> +++ linux-3.0-rc2/net/bridge/br_multicast.c 2011-06-09 20:04:23.473930447 +0900
> @@ -1424,7 +1424,7 @@ static int br_multicast_ipv4_rcv(struct
> switch (ih->type) {
> case IGMP_HOST_MEMBERSHIP_REPORT:
> case IGMPV2_HOST_MEMBERSHIP_REPORT:
> - BR_INPUT_SKB_CB(skb2)->mrouters_only = 1;
> + BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
> err = br_ip4_multicast_add_group(br, port, ih->group);
> break;
> case IGMPV3_HOST_MEMBERSHIP_REPORT:
Acked-by: Stephen Hemminger <shemminger@vyatta.com>
^ permalink raw reply
* Re: [PATCH] net: Ensure tx watchdog failures always print.
From: Eric Dumazet @ 2011-06-14 17:14 UTC (permalink / raw)
To: greearb; +Cc: netdev
In-Reply-To: <1308070213-13244-1-git-send-email-greearb@candelatech.com>
Le mardi 14 juin 2011 à 09:50 -0700, greearb@candelatech.com a écrit :
> From: Ben Greear <greearb@candelatech.com>
>
> These are too important to not have some log message
> generated. But, keep the logic that only prints
> a single stack dump.
>
> Signed-off-by: Ben Greear <greearb@candelatech.com>
> ---
> :100644 100644 90939ba... fce5558... M net/sched/sch_generic.c
> net/sched/sch_generic.c | 16 ++++++++++++++--
> 1 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
> index 90939ba..fce5558 100644
> --- a/net/sched/sch_generic.c
> +++ b/net/sched/sch_generic.c
> @@ -252,8 +252,20 @@ static void dev_watchdog(unsigned long arg)
>
> if (some_queue_timedout) {
> char drivername[64];
> - WARN_ONCE(1, KERN_INFO "NETDEV WATCHDOG: %s (%s): transmit queue %u timed out\n",
> - dev->name, netdev_drivername(dev, drivername, 64), i);
> + static bool do_once = true;
> + printk(KERN_INFO "NETDEV WATCHDOG: %s (%s):"
> + " transmit queue %u timed out,"
> + " trans_start: %lu, wd-timeout: %i"
> + " jiffies: %lu tx-queues: %i\n",
> + dev->name,
> + netdev_drivername(dev, drivername, 64),
> + i,
> + trans_start, dev->watchdog_timeo,
> + jiffies, dev->num_tx_queues);
> + if (do_once) {
> + do_once = false;
> + WARN_ON(1);
You could use WARN_ON_ONCE(1) here
> + }
> dev->netdev_ops->ndo_tx_timeout(dev);
> }
> if (!mod_timer(&dev->watchdog_timer,
^ 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