* Re: [PATCH v3] ipconfig wait for carrier
From: Micha Nelissen @ 2011-05-18 6:59 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20110518.023725.441358922110721042.davem@davemloft.net>
Op 2011-05-18 8:37, David Miller schreef:
> From: Micha Nelissen<micha@neli.hopto.org>
> Date: Wed, 18 May 2011 08:32:35 +0200
>
>> I'm confused. Against which tree/commit do you want it then?
>
> Linus's current tree would be fine as would:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-2.6.git
Ok I see, thanks. The patch should apply just fine to your tree, there
is only a spelling change since 2.6.38 which does not conflict.
Micha
^ permalink raw reply
* Re: small RPS cache for fragments?
From: David Miller @ 2011-05-18 6:37 UTC (permalink / raw)
To: xiaosuo; +Cc: therbert, netdev
In-Reply-To: <BANLkTimmvKn2e5Yrkd5TtktyxTWCRxqinw@mail.gmail.com>
From: Changli Gao <xiaosuo@gmail.com>
Date: Wed, 18 May 2011 07:59:05 +0800
> On Wed, May 18, 2011 at 4:49 AM, David Miller <davem@davemloft.net> wrote:
>>
>> Actually, I think it won't work. Even Linux emits fragments last to
>> first, so we won't see the UDP header until the last packet where it's
>> no longer useful.
>>
>
> No. Linux emits fragments first to last now. You should check the
> current code. :)
I forgot that we rearranged this, thanks :-)
So maybe the original idea can indeed work.
^ permalink raw reply
* Re: [PATCH v3] ipconfig wait for carrier
From: David Miller @ 2011-05-18 6:37 UTC (permalink / raw)
To: micha; +Cc: netdev
In-Reply-To: <4DD36803.1020001@neli.hopto.org>
From: Micha Nelissen <micha@neli.hopto.org>
Date: Wed, 18 May 2011 08:32:35 +0200
> Op 2011-05-18 8:07, David Miller schreef:
>> From: Micha Nelissen<micha@neli.hopto.org>
>> Date: Wed, 18 May 2011 07:22:41 +0200
>>
>>> David, forgive my clumsiness, the previous patch did not compile. This
>>> one applies against 2.6.38 and does compile.
>>
>> Please send patches against the current code, not some arbitrary older
>> source tree.
>
> I'm confused. Against which tree/commit do you want it then?
Linus's current tree would be fine as would:
git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-2.6.git
^ permalink raw reply
* Re: [PATCH v3] ipconfig wait for carrier
From: Micha Nelissen @ 2011-05-18 6:32 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20110518.020716.447947355233874982.davem@davemloft.net>
Op 2011-05-18 8:07, David Miller schreef:
> From: Micha Nelissen<micha@neli.hopto.org>
> Date: Wed, 18 May 2011 07:22:41 +0200
>
>> David, forgive my clumsiness, the previous patch did not compile. This
>> one applies against 2.6.38 and does compile.
>
> Please send patches against the current code, not some arbitrary older
> source tree.
I'm confused. Against which tree/commit do you want it then?
Micha
^ permalink raw reply
* Re: [PATCH net-2.6] net: add skb_dst_force() in sock_queue_err_skb()
From: David Miller @ 2011-05-18 6:21 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, baryluk, shemminger
In-Reply-To: <1305673846.6741.35.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 18 May 2011 01:10:46 +0200
> Commit 7fee226ad239 (add a noref bit on skb dst) forgot to use
> skb_dst_force() on packets queued in sk_error_queue
>
> This triggers following warning, for applications using IP_CMSG_PKTINFO
> receiving one error status
...
> Close bug https://bugzilla.kernel.org/show_bug.cgi?id=34622
>
> Reported-by: Witold Baryluk <baryluk@smp.if.uj.edu.pl>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> CC: Stephen Hemminger <shemminger@vyatta.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH v3] ipconfig wait for carrier
From: David Miller @ 2011-05-18 6:07 UTC (permalink / raw)
To: micha; +Cc: netdev
In-Reply-To: <1305696161-18277-1-git-send-email-micha@neli.hopto.org>
From: Micha Nelissen <micha@neli.hopto.org>
Date: Wed, 18 May 2011 07:22:41 +0200
> David, forgive my clumsiness, the previous patch did not compile. This
> one applies against 2.6.38 and does compile.
Please send patches against the current code, not some arbitrary older
source tree.
Thanks.
^ permalink raw reply
* Re: [PATCH V6 4/6 net-next] vhost: vhost TX zero-copy support
From: Shirley Ma @ 2011-05-18 6:16 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: David Miller, Eric Dumazet, Avi Kivity, Arnd Bergmann, netdev,
kvm, linux-kernel
In-Reply-To: <1305695674.10756.93.camel@localhost.localdomain>
Hello Michael,
Here is the update the patch based on all of your review comments except
the completion/wait for cleanup since I am worried about outstanding
DMAs would prevent vhost from shutting down. I am sending out this for
your review, and test it out later.
For error handling, I update macvtap.c so we can discard the desc even
in zero-copy case.
Signed-off-by: Shirley Ma <xma@us.ibm.com>
---
drivers/vhost/net.c | 42 +++++++++++++++++++++++++++++++++++++++++-
drivers/vhost/vhost.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
drivers/vhost/vhost.h | 13 +++++++++++++
3 files changed, 103 insertions(+), 1 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 2f7c76a..e87a1f8 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -32,6 +32,10 @@
* Using this limit prevents one virtqueue from starving others. */
#define VHOST_NET_WEIGHT 0x80000
+/* MAX number of TX used buffers for outstanding zerocopy */
+#define VHOST_MAX_PEND 128
+#define VHOST_GOODCOPY_LEN PAGE_SIZE
+
enum {
VHOST_NET_VQ_RX = 0,
VHOST_NET_VQ_TX = 1,
@@ -129,6 +133,7 @@ static void handle_tx(struct vhost_net *net)
int err, wmem;
size_t hdr_size;
struct socket *sock;
+ struct skb_ubuf_info pend;
/* TODO: check that we are running from vhost_worker? */
sock = rcu_dereference_check(vq->private_data, 1);
@@ -151,6 +156,10 @@ static void handle_tx(struct vhost_net *net)
hdr_size = vq->vhost_hlen;
for (;;) {
+ /* Release DMAs done buffers first */
+ if (atomic_read(&vq->refcnt) > VHOST_MAX_PEND)
+ vhost_zerocopy_signal_used(vq, false);
+
head = vhost_get_vq_desc(&net->dev, vq, vq->iov,
ARRAY_SIZE(vq->iov),
&out, &in,
@@ -166,6 +175,12 @@ static void handle_tx(struct vhost_net *net)
set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
break;
}
+ /* If more outstanding DMAs, queue the work */
+ if (atomic_read(&vq->refcnt) > VHOST_MAX_PEND) {
+ tx_poll_start(net, sock);
+ set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
+ break;
+ }
if (unlikely(vhost_enable_notify(vq))) {
vhost_disable_notify(vq);
continue;
@@ -188,6 +203,30 @@ static void handle_tx(struct vhost_net *net)
iov_length(vq->hdr, s), hdr_size);
break;
}
+ /* use msg_control to pass vhost zerocopy ubuf info to skb */
+ if (sock_flag(sock->sk, SOCK_ZEROCOPY)) {
+ vq->heads[vq->upend_idx].id = head;
+ if (len < VHOST_GOODCOPY_LEN)
+ /* copy don't need to wait for DMA done */
+ vq->heads[vq->upend_idx].len =
+ VHOST_DMA_DONE_LEN;
+ else {
+ vq->heads[vq->upend_idx].len = len;
+ pend.callback = vhost_zerocopy_callback;
+ pend.arg = vq;
+ pend.desc = vq->upend_idx;
+ msg.msg_control = &pend;
+ msg.msg_controllen = sizeof(pend);
+ }
+ atomic_inc(&vq->refcnt);
+ vq->upend_idx = (vq->upend_idx + 1) % UIO_MAXIOV;
+ /* if upend_idx is full, then wait for free more */
+ if (vq->upend_idx == vq->done_idx) {
+ tx_poll_start(net, sock);
+ set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
+ break;
+ }
+ }
/* TODO: Check specific error and bomb out unless ENOBUFS? */
err = sock->ops->sendmsg(NULL, sock, &msg, len);
if (unlikely(err < 0)) {
@@ -198,7 +237,8 @@ static void handle_tx(struct vhost_net *net)
if (err != len)
pr_debug("Truncated TX packet: "
" len %d != %zd\n", err, len);
- vhost_add_used_and_signal(&net->dev, vq, head, 0);
+ if (!sock_flag(sock->sk, SOCK_ZEROCOPY))
+ vhost_add_used_and_signal(&net->dev, vq, head, 0);
total_len += len;
if (unlikely(total_len >= VHOST_NET_WEIGHT)) {
vhost_poll_queue(&vq->poll);
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 2ab2912..f4c2730 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -174,6 +174,9 @@ static void vhost_vq_reset(struct vhost_dev *dev,
vq->call_ctx = NULL;
vq->call = NULL;
vq->log_ctx = NULL;
+ vq->upend_idx = 0;
+ vq->done_idx = 0;
+ atomic_set(&vq->refcnt, 0);
}
static int vhost_worker(void *data)
@@ -385,16 +388,49 @@ long vhost_dev_reset_owner(struct vhost_dev *dev)
return 0;
}
+/* In case of DMA done not in order in lower device driver for some reason.
+ * upend_idx is used to track end of used idx, done_idx is used to track head
+ * of used idx. Once lower device DMA done contiguously, we will signal KVM
+ * guest used idx.
+ */
+void vhost_zerocopy_signal_used(struct vhost_virtqueue *vq, bool shutdown)
+{
+ int i, j = 0;
+
+ for (i = vq->done_idx; i != vq->upend_idx; i = i++ % UIO_MAXIOV) {
+ if ((vq->heads[i].len == VHOST_DMA_DONE_LEN) || shutdown) {
+ vq->heads[i].len = VHOST_DMA_CLEAR_LEN;
+ vhost_add_used_and_signal(vq->dev, vq,
+ vq->heads[i].id, 0);
+ ++j;
+ } else
+ break;
+ }
+ if (j) {
+ vq->done_idx = i;
+ atomic_sub(j, &vq->refcnt);
+ }
+}
+
/* Caller should have device mutex */
void vhost_dev_cleanup(struct vhost_dev *dev)
{
int i;
+ unsigned long begin = jiffies;
for (i = 0; i < dev->nvqs; ++i) {
if (dev->vqs[i].kick && dev->vqs[i].handle_kick) {
vhost_poll_stop(&dev->vqs[i].poll);
vhost_poll_flush(&dev->vqs[i].poll);
}
+ /* Wait for all lower device DMAs done, then notify virtio_net
+ * or just notify it without waiting for all DMA done here ?
+ * in case of DMAs never done for some reason */
+ if (atomic_read(&dev->vqs[i].refcnt)) {
+ /* how long should we wait ? */
+ msleep(1000);
+ vhost_zerocopy_signal_used(&dev->vqs[i], true);
+ }
if (dev->vqs[i].error_ctx)
eventfd_ctx_put(dev->vqs[i].error_ctx);
if (dev->vqs[i].error)
@@ -603,6 +639,10 @@ static long vhost_set_vring(struct vhost_dev *d, int ioctl, void __user *argp)
mutex_lock(&vq->mutex);
+ /* clean up lower device outstanding DMAs, before setting ring */
+ if (atomic_read(&vq->refcnt))
+ vhost_zerocopy_signal_used(vq, true);
+
switch (ioctl) {
case VHOST_SET_VRING_NUM:
/* Resizing ring with an active backend?
@@ -1416,3 +1456,12 @@ void vhost_disable_notify(struct vhost_virtqueue *vq)
vq_err(vq, "Failed to enable notification at %p: %d\n",
&vq->used->flags, r);
}
+
+void vhost_zerocopy_callback(struct sk_buff *skb)
+{
+ int idx = skb_shinfo(skb)->ubuf.desc;
+ struct vhost_virtqueue *vq = skb_shinfo(skb)->ubuf.arg;
+
+ /* set len = 1 to mark this desc buffers done DMA */
+ vq->heads[idx].len = VHOST_DMA_DONE_LEN;
+}
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index b3363ae..d0e7ac6 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -13,6 +13,11 @@
#include <linux/virtio_ring.h>
#include <asm/atomic.h>
+/* This is for zerocopy, used buffer len is set to 1 when lower device DMA
+ * done */
+#define VHOST_DMA_DONE_LEN 1
+#define VHOST_DMA_CLEAR_LEN 0
+
struct vhost_device;
struct vhost_work;
@@ -108,6 +113,12 @@ struct vhost_virtqueue {
/* Log write descriptors */
void __user *log_base;
struct vhost_log *log;
+ /* vhost zerocopy support */
+ atomic_t refcnt; /* num of outstanding zerocopy DMAs */
+ /* last used idx for outstanding DMA zerocopy buffers */
+ int upend_idx;
+ /* first used idx for DMA done zerocopy buffers */
+ int done_idx;
};
struct vhost_dev {
@@ -154,6 +165,8 @@ bool vhost_enable_notify(struct vhost_virtqueue *);
int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
unsigned int log_num, u64 len);
+void vhost_zerocopy_callback(struct sk_buff *skb);
+void vhost_zerocopy_signal_used(struct vhost_virtqueue *vq, bool shutdown);
#define vq_err(vq, fmt, ...) do { \
pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
^ permalink raw reply related
* Re: [PATCH 09/18] virtio: use avail_event index
From: Michael S. Tsirkin @ 2011-05-18 5:43 UTC (permalink / raw)
To: Rusty Russell
Cc: Krishna Kumar, Carsten Otte, lguest-uLR06cmDAlY/bJ5BZ2RsiQ,
Shirley Ma, kvm-u79uwXL29TY76Z2rM5mHXA,
linux-s390-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
habanero-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, Heiko Carstens,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
virtualization-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
steved-r/Jw6+rmf7HQT0dZR+AlfA, Christian Borntraeger,
Tom Lendacky, Martin Schwidefsky, linux390-tA70FqPdS9bQT0dZR+AlfA
In-Reply-To: <87tycsn9lt.fsf-8n+1lVoiYb80n/F98K4Iww@public.gmane.org>
On Wed, May 18, 2011 at 09:49:42AM +0930, Rusty Russell wrote:
> On Tue, 17 May 2011 09:10:31 +0300, "Michael S. Tsirkin" <mst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > Well one can imagine a driver doing:
> >
> > while (virtqueue_get_buf()) {
> > virtqueue_add_buf()
> > }
> > virtqueue_kick()
> >
> > which looks sensible (batch kicks) but might
> > process any number of bufs between kicks.
>
> No, we currently only expose the buffers in the kick, so it can only
> fill the ring doing that.
>
> We could change that (and maybe that's worth looking at)...
Yes, I think we should - this way host and guest can process
data in parallel without a kick.
My patchset included that simply because it's one index
less to be confused about.
> > If we look at drivers closely enough, I think none
> > of them do the equivalent of the above, but not 100% sure.
>
> I'm pretty sure we don't have this kind of 'echo' driver yet. Drivers
> tend to take OS requests and queue them. The only one which does
> anything even partially sophisticated is the net driver...
>
> Thanks,
> Rusty.
^ permalink raw reply
* [PATCH v3] ipconfig wait for carrier
From: Micha Nelissen @ 2011-05-18 5:22 UTC (permalink / raw)
To: netdev; +Cc: Micha Nelissen, David Miller
In-Reply-To: <1305405386-25187-1-git-send-email-micha@neli.hopto.org>
David, forgive my clumsiness, the previous patch did not compile. This
one applies against 2.6.38 and does compile.
Currently the ip auto configuration has a hardcoded delay of 1 second.
When (ethernet) link takes longer to come up (e.g. more than 3 seconds),
nfs root may not be found.
Remove the hardcoded delay, and wait for carrier on at least one network
device.
Signed-off-by: Micha Nelissen <micha@neli.hopto.org>
Cc: David Miller <davem@davemloft.net>
---
net/ipv4/ipconfig.c | 35 ++++++++++++++++++++++-------------
1 files changed, 22 insertions(+), 13 deletions(-)
diff --git a/net/ipv4/ipconfig.c b/net/ipv4/ipconfig.c
index 2b09775..4225f3f 100644
--- a/net/ipv4/ipconfig.c
+++ b/net/ipv4/ipconfig.c
@@ -87,8 +87,8 @@
#endif
/* Define the friendly delay before and after opening net devices */
-#define CONF_PRE_OPEN 500 /* Before opening: 1/2 second */
-#define CONF_POST_OPEN 1 /* After opening: 1 second */
+#define CONF_POST_OPEN 10 /* After opening: 10 msecs */
+#define CONF_CARRIER_TIMEOUT 120000 /* Wait for carrier timeout */
/* Define the timeout for waiting for a DHCP/BOOTP/RARP reply */
#define CONF_OPEN_RETRIES 2 /* (Re)open devices twice */
@@ -188,14 +188,14 @@ struct ic_device {
static struct ic_device *ic_first_dev __initdata = NULL;/* List of open device */
static struct net_device *ic_dev __initdata = NULL; /* Selected device */
-static bool __init ic_device_match(struct net_device *dev)
+static bool __init ic_is_init_dev(struct net_device *dev)
{
- if (user_dev_name[0] ? !strcmp(dev->name, user_dev_name) :
+ if (dev->flags & IFF_LOOPBACK)
+ return 0;
+ return user_dev_name[0] ? !strcmp(dev->name, user_dev_name) :
(!(dev->flags & IFF_LOOPBACK) &&
(dev->flags & (IFF_POINTOPOINT|IFF_BROADCAST)) &&
- strncmp(dev->name, "dummy", 5)))
- return true;
- return false;
+ strncmp(dev->name, "dummy", 5));
}
static int __init ic_open_devs(void)
@@ -203,6 +203,7 @@ static int __init ic_open_devs(void)
struct ic_device *d, **last;
struct net_device *dev;
unsigned short oflags;
+ unsigned long start;
last = &ic_first_dev;
rtnl_lock();
@@ -216,9 +217,7 @@ static int __init ic_open_devs(void)
}
for_each_netdev(&init_net, dev) {
- if (dev->flags & IFF_LOOPBACK)
- continue;
- if (ic_device_match(dev)) {
+ if (ic_is_init_dev(dev)) {
int able = 0;
if (dev->mtu >= 364)
able |= IC_BOOTP;
@@ -252,6 +251,17 @@ static int __init ic_open_devs(void)
dev->name, able, d->xid));
}
}
+
+ /* wait for a carrier on at least one device */
+ start = jiffies;
+ while (jiffies - start < msecs_to_jiffies(CONF_CARRIER_TIMEOUT)) {
+ for_each_netdev(&init_net, dev)
+ if (ic_is_init_dev(dev) && netif_carrier_ok(dev))
+ goto have_carrier;
+
+ msleep(1);
+ }
+have_carrier:
rtnl_unlock();
*last = NULL;
@@ -1324,14 +1334,13 @@ static int __init wait_for_devices(void)
{
int i;
- msleep(CONF_PRE_OPEN);
for (i = 0; i < DEVICE_WAIT_MAX; i++) {
struct net_device *dev;
int found = 0;
rtnl_lock();
for_each_netdev(&init_net, dev) {
- if (ic_device_match(dev)) {
+ if (ic_is_init_dev(dev)) {
found = 1;
break;
}
@@ -1378,7 +1387,7 @@ static int __init ip_auto_config(void)
return err;
/* Give drivers a chance to settle */
- ssleep(CONF_POST_OPEN);
+ msleep(CONF_POST_OPEN);
/*
* If the config information is insufficient (e.g., our IP address or
--
1.7.4.4
^ permalink raw reply related
* Re: [PATCH V5 4/6 net-next] vhost: vhost TX zero-copy support
From: Shirley Ma @ 2011-05-18 5:14 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: David Miller, Eric Dumazet, Avi Kivity, Arnd Bergmann, netdev,
kvm, linux-kernel
In-Reply-To: <20110517212827.GC7589@redhat.com>
On Wed, 2011-05-18 at 00:28 +0300, Michael S. Tsirkin wrote:
> On Tue, May 17, 2011 at 01:50:19PM -0700, Shirley Ma wrote:
> > Resubmit the patch with most update. This patch passed some
> > live-migration test against RHEL6.2. I will run more stress test w/i
> > live migration.
> >
> > Signed-off-by: Shirley Ma <xma@us.ibm.com>
>
> Cool. cleanup path needs a fix - are you use you can
> not use kobj or some other existing refcounting?
I will look at this.
> Is perf regressiion caused by tx ring overrun gone now?
Nope.
> I added some comments about how we might be aqble
> to complete requests out of order but it's not a must.
Ok.
> > ---
> >
> > drivers/vhost/net.c | 37 +++++++++++++++++++++++++++++++-
> > drivers/vhost/vhost.c | 55
> ++++++++++++++++++++++++++++++++++++++++++++++++-
> > drivers/vhost/vhost.h | 12 ++++++++++
> > 3 files changed, 101 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> > index 2f7c76a..6bd6e28 100644
> > --- a/drivers/vhost/net.c
> > +++ b/drivers/vhost/net.c
> > @@ -32,6 +32,9 @@
> > * Using this limit prevents one virtqueue from starving others. */
> > #define VHOST_NET_WEIGHT 0x80000
> >
> > +/* MAX number of TX used buffers for outstanding zerocopy */
> > +#define VHOST_MAX_ZEROCOPY_PEND 128
> > +
> > enum {
> > VHOST_NET_VQ_RX = 0,
> > VHOST_NET_VQ_TX = 1,
> > @@ -129,6 +132,7 @@ static void handle_tx(struct vhost_net *net)
> > int err, wmem;
> > size_t hdr_size;
> > struct socket *sock;
> > + struct skb_ubuf_info pend;
> >
> > /* TODO: check that we are running from vhost_worker? */
> > sock = rcu_dereference_check(vq->private_data, 1);
> > @@ -151,6 +155,10 @@ static void handle_tx(struct vhost_net *net)
> > hdr_size = vq->vhost_hlen;
> >
> > for (;;) {
> > + /* Release DMAs done buffers first */
> > + if (atomic_read(&vq->refcnt) >
> VHOST_MAX_ZEROCOPY_PEND)
> > + vhost_zerocopy_signal_used(vq, false);
> > +
> > head = vhost_get_vq_desc(&net->dev, vq, vq->iov,
> > ARRAY_SIZE(vq->iov),
> > &out, &in,
> > @@ -166,6 +174,13 @@ static void handle_tx(struct vhost_net *net)
> > set_bit(SOCK_ASYNC_NOSPACE,
> &sock->flags);
> > break;
> > }
> > + /* If more outstanding DMAs, queue the work */
> > + if (sock_flag(sock->sk, SOCK_ZEROCOPY) &&
> > + (atomic_read(&vq->refcnt) >
> VHOST_MAX_ZEROCOPY_PEND)) {
> > + tx_poll_start(net, sock);
> > + set_bit(SOCK_ASYNC_NOSPACE,
> &sock->flags);
> > + break;
> > + }
> > if (unlikely(vhost_enable_notify(vq))) {
> > vhost_disable_notify(vq);
> > continue;
> > @@ -188,17 +203,35 @@ static void handle_tx(struct vhost_net *net)
> > iov_length(vq->hdr, s), hdr_size);
> > break;
> > }
> > + /* use msg_control to pass vhost zerocopy ubuf info to
> skb */
> > + if (sock_flag(sock->sk, SOCK_ZEROCOPY)) {
> > + vq->heads[vq->upend_idx].id = head;
> > + if (len <= 128)
>
> I thought we have a constant for that?
Yes, fixed it already. I might change it to PAGE_SIZE for now since the
small message sizes regression issue.
> > + vq->heads[vq->upend_idx].len =
> VHOST_DMA_DONE_LEN;
> > + else {
> > + vq->heads[vq->upend_idx].len = len;
> > + pend.callback =
> vhost_zerocopy_callback;
> > + pend.arg = vq;
> > + pend.desc = vq->upend_idx;
> > + msg.msg_control = &pend;
> > + msg.msg_controllen = sizeof(pend);
> > + }
> > + atomic_inc(&vq->refcnt);
> > + vq->upend_idx = (vq->upend_idx + 1) %
> UIO_MAXIOV;
>
> Ok, so we deal with a cyclic ring apparently? What guarantees we don't
> overrun it?
VHOST_MAX_PEND (which is 128) should prevent it from overrun normally.
In the case of none DMAs can be done, the maximum entries are the ring
size, which is much smaller or equal to UIO_MAXIOV for current
implementation.
Maybe I should add some condition check if it is overrun then exits?
>
> > + }
> > /* TODO: Check specific error and bomb out unless
> ENOBUFS? */
> > err = sock->ops->sendmsg(NULL, sock, &msg, len);
> > if (unlikely(err < 0)) {
> > - vhost_discard_vq_desc(vq, 1);
> > + if (!sock_flag(sock->sk, SOCK_ZEROCOPY))
> > + vhost_discard_vq_desc(vq, 1);
>
> How are errors handled with zerocopy?
I thought about it before: if error after macvtap skb is allocated, then
that skb has a callback which will set DMA done for add_used, if the
error before macvtap skb is allocated, then it can reverse as copy case.
To avoid the complexity check, I decided to not handle it.
>
> > tx_poll_start(net, sock);
> > break;
> > }
> > if (err != len)
> > pr_debug("Truncated TX packet: "
> > " len %d != %zd\n", err, len);
> > - vhost_add_used_and_signal(&net->dev, vq, head, 0);
> > + if (!sock_flag(sock->sk, SOCK_ZEROCOPY))
> > + vhost_add_used_and_signal(&net->dev, vq, head,
> 0);
> > total_len += len;
> > if (unlikely(total_len >= VHOST_NET_WEIGHT)) {
> > vhost_poll_queue(&vq->poll);
> > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > index 2ab2912..ce799d6 100644
> > --- a/drivers/vhost/vhost.c
> > +++ b/drivers/vhost/vhost.c
> > @@ -174,6 +174,9 @@ static void vhost_vq_reset(struct vhost_dev
> *dev,
> > vq->call_ctx = NULL;
> > vq->call = NULL;
> > vq->log_ctx = NULL;
> > + vq->upend_idx = 0;
> > + vq->done_idx = 0;
> > + atomic_set(&vq->refcnt, 0);
> > }
> >
> > static int vhost_worker(void *data)
> > @@ -230,7 +233,7 @@ static long vhost_dev_alloc_iovecs(struct
> vhost_dev *dev)
> > UIO_MAXIOV,
> GFP_KERNEL);
> > dev->vqs[i].log = kmalloc(sizeof *dev->vqs[i].log *
> UIO_MAXIOV,
> > GFP_KERNEL);
> > - dev->vqs[i].heads = kmalloc(sizeof *dev->vqs[i].heads
> *
> > + dev->vqs[i].heads = kzalloc(sizeof *dev->vqs[i].heads
> *
> > UIO_MAXIOV, GFP_KERNEL);
>
> Which fields need to be initialized actually?
Nope, already fixed it with kmalloc.
> >
> > if (!dev->vqs[i].indirect || !dev->vqs[i].log ||
> > @@ -385,6 +388,38 @@ long vhost_dev_reset_owner(struct vhost_dev
> *dev)
> > return 0;
> > }
> >
> > +/*
> > + comments
> > +*/
>
> Hmm.
Fixed it in previous patch already.
> > +void vhost_zerocopy_signal_used(struct vhost_virtqueue *vq, bool
> shutdown)
> > +{
> > + int i, j = 0;
> > +
> > + i = vq->done_idx;
> > + while (i != vq->upend_idx) {
>
> A for loop might be clearer.
Ok.
> > + if ((vq->heads[i].len == VHOST_DMA_DONE_LEN) ||
> shutdown) {
>
> On shutdown, we signal all buffers used to the guest?
> Why?
We signal all outstand DMAs in the case of driver has some DMA issue to
prevent us from shutting down. I am afraid vhost cleanup could wait
forever.
> > + /* reset len = 0 */
>
> comment not very helpful.
> Could you explain what this does instead?
> Or better use some constant instead of 0 ...
Fixed it already.
> > + vq->heads[i].len = 0;
> > + i = (i + 1) % UIO_MAXIOV;
> > + ++j;
> > + } else
> > + break;
>
> Hmm so if the 1st entry does not complete, you do not signal anything?
No used buffers to guest, should we signal still?
> > + }
>
> Looking at this loop, done idx is the consumer and used idx
> is the producer, right?
Yes.
> > + if (j) {
> > + /* comments */
>
> Yes?
>
> > + if (i > vq->done_idx)
> > + vhost_add_used_n(vq, &vq->heads[vq->done_idx],
> j);
> > + else {
> > + vhost_add_used_n(vq, &vq->heads[vq->done_idx],
> > + UIO_MAXIOV - vq->done_idx);
> > + vhost_add_used_n(vq, vq->heads, i);
> > + }
> > + vq->done_idx = i;
> > + vhost_signal(vq->dev, vq);
> > + atomic_sub(j, &vq->refcnt);
>
> Code will likely be simpler if you call vhost_add_used once for
> each head in the first loop. Possibly add_used_signal might be
> a good idea too.
Ok.
> > + }
> > +}
> > +
> > /* Caller should have device mutex */
> > void vhost_dev_cleanup(struct vhost_dev *dev)
> > {
> > @@ -395,6 +430,11 @@ void vhost_dev_cleanup(struct vhost_dev *dev)
> > vhost_poll_stop(&dev->vqs[i].poll);
> > vhost_poll_flush(&dev->vqs[i].poll);
> > }
> > + /* wait for all lower device DMAs done, then notify
> guest */
> > + if (atomic_read(&dev->vqs[i].refcnt)) {
> > + msleep(1000);
> > + vhost_zerocopy_signal_used(&dev->vqs[i],
> true);
> > + }
>
> This needs to be fixed somehow. Use a completion object and wait
> on it?
Worried about what if the driver has some DMAs issue, which would
prevent vhost from shutting down.
> > if (dev->vqs[i].error_ctx)
> > eventfd_ctx_put(dev->vqs[i].error_ctx);
> > if (dev->vqs[i].error)
> > @@ -603,6 +643,10 @@ static long vhost_set_vring(struct vhost_dev
> *d, int ioctl, void __user *argp)
> >
> > mutex_lock(&vq->mutex);
> >
> > + /* force all lower device DMAs done */
> > + if (atomic_read(&vq->refcnt))
> > + vhost_zerocopy_signal_used(vq, true);
> > +
> > switch (ioctl) {
> > case VHOST_SET_VRING_NUM:
> > /* Resizing ring with an active backend?
> > @@ -1416,3 +1460,12 @@ void vhost_disable_notify(struct
> vhost_virtqueue *vq)
> > vq_err(vq, "Failed to enable notification at %p: %d
> \n",
> > &vq->used->flags, r);
> > }
> > +
> > +void vhost_zerocopy_callback(struct sk_buff *skb)
> > +{
> > + int idx = skb_shinfo(skb)->ubuf.desc;
> > + struct vhost_virtqueue *vq = skb_shinfo(skb)->ubuf.arg;
> > +
> > + /* set len = 1 to mark this desc buffers done DMA */
>
> this comment can now go.
Yes, it's gone already.
> > + vq->heads[idx].len = VHOST_DMA_DONE_LEN;
> > +}
> > diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> > index b3363ae..8e3ecc7 100644
> > --- a/drivers/vhost/vhost.h
> > +++ b/drivers/vhost/vhost.h
> > @@ -13,6 +13,10 @@
> > #include <linux/virtio_ring.h>
> > #include <asm/atomic.h>
> >
> > +/* This is for zerocopy, used buffer len is set to 1 when lower
> device DMA
> > + * done */
> > +#define VHOST_DMA_DONE_LEN 1
> > +
> > struct vhost_device;
> >
> > struct vhost_work;
> > @@ -108,6 +112,12 @@ struct vhost_virtqueue {
> > /* Log write descriptors */
> > void __user *log_base;
> > struct vhost_log *log;
> > + /* vhost zerocopy support */
> > + atomic_t refcnt; /* num of outstanding zerocopy DMAs */
>
> future enhancement idea: this is used apparently under vq lock
> so no need for an atomic?
It is also used in skb vhost zerocopy callback.
> > + /* copy of avail idx to monitor outstanding DMA zerocopy
> buffers */
>
> looking at code upend_idx seems to be calculated independently
> of guest avail idx - could you clarify pls?
Yes, you are right. Should change it to: upend_idx is used to track
vring ids for outstanding zero-copy DMA buffers?
> > + int upend_idx;
> > + /* copy of used idx to monintor DMA done zerocopy buffers */
>
> monitor
Ok.
> > + int done_idx;
>
>
> I think in reality these are just producer and consumer
> in the head structure which for zero copy is used
Yes.
>
> > };
> >
> > struct vhost_dev {
> > @@ -154,6 +164,8 @@ bool vhost_enable_notify(struct vhost_virtqueue
> *);
> >
> > int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log
> *log,
> > unsigned int log_num, u64 len);
> > +void vhost_zerocopy_callback(struct sk_buff *skb);
> > +void vhost_zerocopy_signal_used(struct vhost_virtqueue *vq, bool
> shutdown);
> >
> > #define vq_err(vq, fmt, ...) do {
> \
> > pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
> >
^ permalink raw reply
* Re: [PATCH] Check the value of doi before referencing it
From: David Miller @ 2011-05-18 5:04 UTC (permalink / raw)
To: huzaifas; +Cc: netdev, kaber, yoshfuji, jmorris, pekkas, kuznet
In-Reply-To: <1305692980-4730-1-git-send-email-huzaifas@redhat.com>
From: Huzaifa Sidhpurwala <huzaifas@redhat.com>
Date: Wed, 18 May 2011 09:59:40 +0530
> Value of doi is not checked before referencing it.
> Though this does not cause any null pointer dereference since
> all the callers of cipso_v4_doi_add check the value of doi
> before calling the function, but it would be a good programming
> practice to do so anyways :)
>
> Signed-off-by: Huzaifa Sidhpurwala <huzaifas@redhat.com>
I don't think we should fix bugs that do not exist.
^ permalink raw reply
* [PATCH 2/2] bna: Add Generic Netlink Interface
From: Rasesh Mody @ 2011-05-18 4:57 UTC (permalink / raw)
To: davem, netdev; +Cc: adapter_linux_open_src_team, Rasesh Mody, Debashis Dutt
In-Reply-To: <1305694621-28023-1-git-send-email-rmody@brocade.com>
This patch adds the generic netlink communication interface to BNA driver. The
in-kernel generic netlink infrastructure can be used to collect hardware
specific control information and control attributes. The driver makes use of
the "doit" handler provided by the generic netlink layer to accomplish this.
Signed-off-by: Debashis Dutt <ddutt@brocade.com>
Signed-off-by: Rasesh Mody <rmody@brocade.com>
---
drivers/net/bna/Makefile | 2 +-
drivers/net/bna/bfa_defs.h | 18 +++
drivers/net/bna/bfa_ioc.c | 8 +-
drivers/net/bna/bfa_ioc.h | 1 +
drivers/net/bna/bnad.c | 11 ++-
drivers/net/bna/bnad.h | 1 +
drivers/net/bna/bnad_genl.c | 345 +++++++++++++++++++++++++++++++++++++++++++
drivers/net/bna/bnad_genl.h | 87 +++++++++++
8 files changed, 466 insertions(+), 7 deletions(-)
create mode 100644 drivers/net/bna/bnad_genl.c
create mode 100644 drivers/net/bna/bnad_genl.h
diff --git a/drivers/net/bna/Makefile b/drivers/net/bna/Makefile
index 4bb0d5d..f3339dc 100644
--- a/drivers/net/bna/Makefile
+++ b/drivers/net/bna/Makefile
@@ -5,7 +5,7 @@
obj-$(CONFIG_BNA) += bna.o
-bna-objs := bnad.o bnad_debugfs.o bnad_ethtool.o
+bna-objs := bnad.o bnad_debugfs.o bnad_ethtool.o bnad_genl.o
bna-objs += bna_ctrl.o bna_txrx.o
bna-objs += bfa_ioc.o bfa_ioc_ct.o bfa_cee.o cna_fwimg.o
diff --git a/drivers/net/bna/bfa_defs.h b/drivers/net/bna/bfa_defs.h
index 2ea0dfe..2dd0898 100644
--- a/drivers/net/bna/bfa_defs.h
+++ b/drivers/net/bna/bfa_defs.h
@@ -26,6 +26,24 @@
#define BFA_STRING_32 32
#define BFA_VERSION_LEN 64
+/*
+ * Check if the card having old wwn/mac handling
+ */
+#define bfa_mfg_is_old_wwn_mac_model(type) (( \
+ (type) == BFA_MFG_TYPE_CNA10P2 || \
+ (type) == BFA_MFG_TYPE_CNA10P1 || \
+ (type) == BFA_MFG_TYPE_WANCHESE))
+
+#define bfa_mfg_increment_wwn_mac(m, i) \
+do { \
+ u32 t = ((u32)(m)[0] << 16) | ((u32)(m)[1] << 8) | \
+ (u32)(m)[2]; \
+ t += (i); \
+ (m)[0] = (t >> 16) & 0xFF; \
+ (m)[1] = (t >> 8) & 0xFF; \
+ (m)[2] = t & 0xFF; \
+} while (0)
+
/**
* ---------------------- adapter definitions ------------
*/
diff --git a/drivers/net/bna/bfa_ioc.c b/drivers/net/bna/bfa_ioc.c
index 15f9dec..eeb7250 100644
--- a/drivers/net/bna/bfa_ioc.c
+++ b/drivers/net/bna/bfa_ioc.c
@@ -82,8 +82,6 @@ static void bfa_ioc_pf_fwmismatch(struct bfa_ioc *ioc);
static void bfa_ioc_boot(struct bfa_ioc *ioc, u32 boot_type,
u32 boot_param);
static u32 bfa_ioc_smem_pgnum(struct bfa_ioc *ioc, u32 fmaddr);
-static void bfa_ioc_get_adapter_serial_num(struct bfa_ioc *ioc,
- char *serial_num);
static void bfa_ioc_get_adapter_fw_ver(struct bfa_ioc *ioc,
char *fw_ver);
static void bfa_ioc_get_pci_chip_rev(struct bfa_ioc *ioc,
@@ -2045,7 +2043,7 @@ bfa_ioc_get_adapter_attr(struct bfa_ioc *ioc,
ioc_attr = ioc->attr;
- bfa_ioc_get_adapter_serial_num(ioc, ad_attr->serial_num);
+ bfa_nw_ioc_get_adapter_serial_num(ioc, ad_attr->serial_num);
bfa_ioc_get_adapter_fw_ver(ioc, ad_attr->fw_ver);
bfa_ioc_get_adapter_optrom_ver(ioc, ad_attr->optrom_ver);
bfa_ioc_get_adapter_manufacturer(ioc, ad_attr->manufacturer);
@@ -2096,8 +2094,8 @@ bfa_ioc_get_type(struct bfa_ioc *ioc)
}
}
-static void
-bfa_ioc_get_adapter_serial_num(struct bfa_ioc *ioc, char *serial_num)
+void
+bfa_nw_ioc_get_adapter_serial_num(struct bfa_ioc *ioc, char *serial_num)
{
memset(serial_num, 0, BFA_ADAPTER_SERIAL_NUM_LEN);
memcpy(serial_num,
diff --git a/drivers/net/bna/bfa_ioc.h b/drivers/net/bna/bfa_ioc.h
index 49739cd..1f71865 100644
--- a/drivers/net/bna/bfa_ioc.h
+++ b/drivers/net/bna/bfa_ioc.h
@@ -280,6 +280,7 @@ mac_t bfa_nw_ioc_get_mac(struct bfa_ioc *ioc);
void bfa_nw_ioc_debug_memclaim(struct bfa_ioc *ioc, void *dbg_fwsave);
int bfa_nw_ioc_debug_fwtrc(struct bfa_ioc *ioc, void *trcdata, int *trclen);
int bfa_nw_ioc_debug_fwsave(struct bfa_ioc *ioc, void *trcdata, int *trclen);
+void bfa_nw_ioc_get_adapter_serial_num(struct bfa_ioc *ioc, char *serial_num);
/*
* Timeout APIs
diff --git a/drivers/net/bna/bnad.c b/drivers/net/bna/bnad.c
index a997276..09aa132 100644
--- a/drivers/net/bna/bnad.c
+++ b/drivers/net/bna/bnad.c
@@ -25,6 +25,7 @@
#include <linux/ip.h>
#include "bnad.h"
+#include "bnad_genl.h"
#include "bna.h"
#include "cna.h"
@@ -721,7 +722,7 @@ bnad_disable_mbox_irq(struct bnad *bnad)
BNAD_UPDATE_CTR(bnad, mbox_intr_disabled);
}
-static void
+void
bnad_set_netdev_perm_addr(struct bnad *bnad)
{
struct net_device *netdev = bnad->netdev;
@@ -3296,6 +3297,10 @@ bnad_module_init(void)
return err;
}
+ /* Register with generic netlink */
+ if (bnad_genl_init())
+ pr_err("bna: Generic Netlink Register failed\n");
+
return 0;
}
@@ -3305,6 +3310,10 @@ bnad_module_exit(void)
pci_unregister_driver(&bnad_pci_driver);
mutex_destroy(&bnad_list_mutex);
+ /* Unegister with generic netlink */
+ if (bnad_genl_uninit())
+ pr_err("bna: Generic Netlink Unregister failed\n");
+
if (bfi_fw)
release_firmware(bfi_fw);
}
diff --git a/drivers/net/bna/bnad.h b/drivers/net/bna/bnad.h
index 2c1f283..d3cb27b 100644
--- a/drivers/net/bna/bnad.h
+++ b/drivers/net/bna/bnad.h
@@ -302,6 +302,7 @@ extern u32 *cna_get_firmware_buf(struct pci_dev *pdev);
extern void bnad_set_ethtool_ops(struct net_device *netdev);
/* Configuration & setup */
+extern void bnad_set_netdev_perm_addr(struct bnad *bnad);
extern void bnad_tx_coalescing_timeo_set(struct bnad *bnad);
extern void bnad_rx_coalescing_timeo_set(struct bnad *bnad);
diff --git a/drivers/net/bna/bnad_genl.c b/drivers/net/bna/bnad_genl.c
new file mode 100644
index 0000000..eec2a56
--- /dev/null
+++ b/drivers/net/bna/bnad_genl.c
@@ -0,0 +1,345 @@
+/*
+ * Linux network driver for Brocade Converged Network Adapter.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License (GPL) Version 2 as
+ * published by the Free Software Foundation
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ */
+/*
+ * Copyright (c) 2005-2011 Brocade Communications Systems, Inc.
+ * All rights reserved
+ * www.brocade.com
+ */
+#include "bnad.h"
+#include "bnad_genl.h"
+#include "bna.h"
+
+static struct nla_policy bnad_genl_policy[BNAD_GENL_ATTR_MAX + 1] = {
+ [BNAD_GENL_ATTR_IOC_INFO]
+ = { .len = sizeof(struct bnad_genl_ioc_info) },
+ [BNAD_GENL_ATTR_IOC_ATTR]
+ = { .len = sizeof(struct bnad_genl_ioc_attr) },
+};
+
+static struct genl_family bnad_genl_family = {
+ .id = GENL_ID_GENERATE,
+ .name = "BNAD_GENL",
+ .version = BNAD_GENL_VERSION,
+ .hdrsize = 0,
+ .maxattr = BNAD_GENL_ATTR_MAX,
+};
+
+static struct bnad *
+bnad_get_bnadev(int bna_id)
+{
+ struct bnad *bnad;
+
+ mutex_lock(&bnad_list_mutex);
+ list_for_each_entry(bnad, &bnad_list, list_entry) {
+ if (bnad->id == bna_id) {
+ mutex_unlock(&bnad_list_mutex);
+ return bnad;
+ }
+ }
+ mutex_unlock(&bnad_list_mutex);
+ return NULL;
+}
+
+static void
+bnad_hwpath_get(struct bnad *bnad, char *hwpath, char *adapter_hwpath)
+{
+ int i;
+
+ strcpy(hwpath, pci_name(bnad->pcidev));
+ strcpy(adapter_hwpath, pci_name(bnad->pcidev));
+ i = strlen(adapter_hwpath) - 1;
+ while (i && (adapter_hwpath[i] != '.'))
+ i--;
+ adapter_hwpath[i] = '\0';
+}
+
+static void
+bnad_get_pci_attr(struct bnad *bnad, struct bfa_ioc_pci_attr *pci_attr)
+{
+ pci_attr->vendor_id = bnad->pcidev->vendor;
+ pci_attr->device_id = bnad->pcidev->device;
+ pci_attr->ssid = bnad->pcidev->subsystem_device;
+ pci_attr->ssvid = bnad->pcidev->subsystem_vendor;
+ pci_attr->pcifn = PCI_FUNC(bnad->pcidev->devfn);
+}
+
+static int
+bnad_genl_ioc_info_rsp(struct genl_info *info,
+ struct bnad_genl_ioc_info *genlcmd, size_t attr_size, u8 cmd)
+{
+ struct sk_buff *rsp_skb = NULL;
+ void *genl_msg_hdr = NULL;
+ size_t msg_size;
+ int err = 0;
+
+ msg_size = nla_total_size(attr_size);
+
+ rsp_skb = genlmsg_new(msg_size, GFP_KERNEL);
+ if (!rsp_skb) {
+ pr_warn("bnad[%d]: Failed to get response skb\n",
+ genlcmd->bnad_num);
+ return -ENOMEM;
+ }
+
+ genl_msg_hdr = genlmsg_put(rsp_skb, info->snd_pid, info->snd_seq,
+ &bnad_genl_family, 0, cmd);
+ if (!genl_msg_hdr) {
+ pr_warn("bnad[%d]: Failed to get the genl_msg_header\n",
+ genlcmd->bnad_num);
+ err = -ENOMEM;
+ goto failure;
+ }
+
+ NLA_PUT(rsp_skb, BNAD_GENL_ATTR_IOC_INFO, attr_size, genlcmd);
+
+ err = genlmsg_end(rsp_skb, genl_msg_hdr);
+ if (err < 0) {
+ pr_warn("bnad[%d]: Failed to do genlmsg_end\n",
+ genlcmd->bnad_num);
+ goto failure;
+ }
+
+ err = genlmsg_reply(rsp_skb, info);
+ if (err)
+ pr_warn("bnad[%d]: Could not do genlmsg_reply (%d)\n",
+ genlcmd->bnad_num, err);
+
+ return err;
+
+nla_put_failure:
+ pr_warn("bnad[%d]: Failed to do NLA_PUT\n", genlcmd->bnad_num);
+ err = -EMSGSIZE;
+failure:
+ genlmsg_cancel(rsp_skb, genl_msg_hdr);
+ kfree_skb(rsp_skb);
+ return err;
+}
+
+static int
+bnad_genl_ioc_attr_rsp(struct genl_info *info,
+ struct bnad_genl_ioc_attr *genlcmd, size_t attr_size, u8 cmd)
+{
+ struct sk_buff *rsp_skb = NULL;
+ void *genl_msg_hdr = NULL;
+ size_t msg_size;
+ int err = 0;
+
+ msg_size = nla_total_size(attr_size);
+
+ rsp_skb = genlmsg_new(msg_size, GFP_KERNEL);
+ if (!rsp_skb) {
+ pr_warn("bnad[%d]: Failed to get response skb\n",
+ genlcmd->bnad_num);
+ return -ENOMEM;
+ }
+
+ genl_msg_hdr = genlmsg_put(rsp_skb, info->snd_pid, info->snd_seq,
+ &bnad_genl_family, 0, cmd);
+ if (!genl_msg_hdr) {
+ pr_warn("bnad[%d]: Failed to get the genl_msg_header\n",
+ genlcmd->bnad_num);
+ err = -ENOMEM;
+ goto failure;
+ }
+
+ NLA_PUT(rsp_skb, BNAD_GENL_ATTR_IOC_ATTR, attr_size, genlcmd);
+
+ err = genlmsg_end(rsp_skb, genl_msg_hdr);
+ if (err < 0) {
+ pr_warn("bnad[%d]: Failed to do genlmsg_end\n",
+ genlcmd->bnad_num);
+ goto failure;
+ }
+
+ err = genlmsg_reply(rsp_skb, info);
+ if (err)
+ pr_warn("bnad[%d]: Could not do genlmsg_reply (%d)\n",
+ genlcmd->bnad_num, err);
+
+ return err;
+
+nla_put_failure:
+ pr_warn("bnad[%d]: Failed to do NLA_PUT\n", genlcmd->bnad_num);
+ err = -EMSGSIZE;
+failure:
+ genlmsg_cancel(rsp_skb, genl_msg_hdr);
+ kfree_skb(rsp_skb);
+ return err;
+}
+
+/* Note: Should be called holding bnad_conf_lock */
+static int
+bnad_genl_ioc_get_info(struct sk_buff *skb, struct genl_info *info)
+{
+ struct bnad *bnad = NULL;
+ struct bnad_genl_ioc_info *genlcmd = NULL;
+ struct bfa_ioc *ioc = NULL;
+ unsigned long flags = 0;
+ int err;
+
+ genlcmd = (struct bnad_genl_ioc_info *)
+ nla_data(info->attrs[BNAD_GENL_ATTR_IOC_INFO]);
+
+ bnad = bnad_get_bnadev(genlcmd->bnad_num);
+ if (!bnad) {
+ pr_warn("bna: Failed to get driver instance\n");
+ return -EINVAL;
+ }
+ ioc = &bnad->bna.device.ioc;
+
+ spin_lock_irqsave(&bnad->bna_lock, flags);
+ bfa_nw_ioc_get_adapter_serial_num(ioc, genlcmd->serialnum);
+ genlcmd->mac = bfa_nw_ioc_get_mac(ioc);
+ /* Get manufacturing MAC */
+ genlcmd->factory_mac = ioc->attr->mfg_mac;
+ if (bfa_mfg_is_old_wwn_mac_model(ioc->attr->card_type))
+ genlcmd->factory_mac.mac[MAC_ADDRLEN - 1] += bfa_ioc_pcifn(ioc);
+ else
+ bfa_mfg_increment_wwn_mac(
+ &(genlcmd->factory_mac.mac[MAC_ADDRLEN-3]),
+ bfa_ioc_pcifn(ioc));
+
+ /* Get stack MAC */
+ if (is_zero_ether_addr(&bnad->perm_addr.mac[0])) {
+ bna_port_mac_get(&bnad->bna.port, &bnad->perm_addr);
+ bnad_set_netdev_perm_addr(bnad);
+ }
+ memcpy(&genlcmd->current_mac, bnad->netdev->dev_addr, sizeof(mac_t));
+
+ genlcmd->bnad_num = bnad->id;
+ spin_unlock_irqrestore(&bnad->bna_lock, flags);
+ bnad_hwpath_get(bnad, genlcmd->hwpath, genlcmd->adapter_hwpath);
+ sprintf(genlcmd->eth_name, "%s", bnad->netdev->name);
+ strncpy(genlcmd->name, bnad->adapter_name, sizeof(genlcmd->name) - 1);
+ strncpy(genlcmd->port_name, bnad->port_name,
+ sizeof(genlcmd->port_name) - 1);
+ genlcmd->ioc_type = BFA_IOC_TYPE_LL;
+ genlcmd->status = BFA_STATUS_OK;
+
+ err = bnad_genl_ioc_info_rsp(info, genlcmd,
+ sizeof(struct bnad_genl_ioc_info), BNAD_GENL_CMD_IOC_INFO);
+
+ return err;
+}
+
+/* Note: Should be called holding bnad_conf_lock */
+static int
+bnad_genl_ioc_get_attr(struct sk_buff *skb, struct genl_info *info)
+{
+ struct bnad *bnad = NULL;
+ struct bnad_genl_ioc_attr *genlcmd = NULL;
+ struct bfa_ioc *ioc = NULL;
+ unsigned long flags = 0;
+ int err = 0;
+
+ genlcmd = (struct bnad_genl_ioc_attr *)
+ nla_data(info->attrs[BNAD_GENL_ATTR_IOC_ATTR]);
+
+ bnad = bnad_get_bnadev(genlcmd->bnad_num);
+ if (!bnad) {
+ pr_warn("bna: Failed to get driver instance\n");
+ return -EINVAL;
+ }
+ ioc = &bnad->bna.device.ioc;
+
+ memset(&genlcmd->ioc_attr, 0, sizeof(genlcmd->ioc_attr));
+ spin_lock_irqsave(&bnad->bna_lock, flags);
+ bfa_nw_ioc_get_attr(&bnad->bna.device.ioc, &genlcmd->ioc_attr);
+ spin_unlock_irqrestore(&bnad->bna_lock, flags);
+ genlcmd->ioc_attr.ioc_type = BFA_IOC_TYPE_LL;
+ strcpy(genlcmd->ioc_attr.driver_attr.driver, BNAD_NAME);
+ strncpy(genlcmd->ioc_attr.driver_attr.driver_ver,
+ BNAD_VERSION, BFA_VERSION_LEN);
+ strcpy(genlcmd->ioc_attr.driver_attr.fw_ver,
+ genlcmd->ioc_attr.adapter_attr.fw_ver);
+ strcpy(genlcmd->ioc_attr.driver_attr.bios_ver,
+ genlcmd->ioc_attr.adapter_attr.optrom_ver);
+ bnad_get_pci_attr(bnad, &genlcmd->ioc_attr.pci_attr);
+ genlcmd->status = BFA_STATUS_OK;
+
+ err = bnad_genl_ioc_attr_rsp(info, genlcmd,
+ sizeof(struct bnad_genl_ioc_attr), BNAD_GENL_CMD_IOC_ATTR);
+
+ return err;
+}
+
+/* BNAD generic netlink ops */
+static struct genl_ops bnad_genl_ops[] = {
+ {
+ .cmd = BNAD_GENL_CMD_IOC_INFO,
+ .flags = 0,
+ .policy = bnad_genl_policy,
+ .doit = bnad_genl_ioc_get_info,
+ .dumpit = NULL,
+ },
+ {
+ .cmd = BNAD_GENL_CMD_IOC_ATTR,
+ .flags = 0,
+ .policy = bnad_genl_policy,
+ .doit = bnad_genl_ioc_get_attr,
+ .dumpit = NULL,
+ },
+};
+
+/* Initialize generic netlink for BNAD */
+int
+bnad_genl_init(void)
+{
+ int i, err = 0;
+
+ /* Register family */
+ err = genl_register_family(&bnad_genl_family);
+ if (err) {
+ pr_warn("bna: failed to register with Netlink\n");
+ return err;
+ }
+ pr_info("bna: registered with Netlink\n");
+
+ /* Register ops */
+ for (i = 0; i < sizeof(bnad_genl_ops) / sizeof(bnad_genl_ops[0]); i++) {
+ err = genl_register_ops(&bnad_genl_family, &bnad_genl_ops[i]);
+ if (err)
+ pr_warn("bna: failed to register netlink op %u\n",
+ bnad_genl_ops[i].cmd);
+ else
+ pr_info("bna: registered netlink op %u\n",
+ bnad_genl_ops[i].cmd);
+ }
+
+ return err;
+}
+
+/* Uninitialize generic netlink for BNAD */
+int
+bnad_genl_uninit(void)
+{
+ int i, err = 0;
+
+ for (i = 0; i < sizeof(bnad_genl_ops) / sizeof(bnad_genl_ops[0]); i++) {
+ err = genl_unregister_ops(&bnad_genl_family, &bnad_genl_ops[i]);
+ if (err)
+ pr_warn("bna: failed to unregister netlink op %u)\n",
+ bnad_genl_ops[i].cmd);
+ else
+ pr_info("bna: unregistered netlink op %u\n",
+ bnad_genl_ops[i].cmd);
+ }
+
+ err = genl_unregister_family(&bnad_genl_family);
+ if (err)
+ pr_warn("bna: failed to unregister with Netlink\n");
+ else
+ pr_info("bna: unregistered with Netlink\n");
+
+ return err;
+}
diff --git a/drivers/net/bna/bnad_genl.h b/drivers/net/bna/bnad_genl.h
new file mode 100644
index 0000000..d469fe5
--- /dev/null
+++ b/drivers/net/bna/bnad_genl.h
@@ -0,0 +1,87 @@
+/*
+ * Linux network driver for Brocade Converged Network Adapter.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License (GPL) Version 2 as
+ * published by the Free Software Foundation
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ */
+/*
+ * Copyright (c) 2005-2011 Brocade Communications Systems, Inc.
+ * All rights reserved
+ * www.brocade.com
+ */
+#ifndef __BNAD_GENL_H__
+#define __BNAD_GENL_H__
+
+#include <linux/device.h>
+#include <linux/netdevice.h>
+#include <linux/gfp.h>
+#include <linux/skbuff.h>
+#include <linux/ethtool.h>
+#include <linux/rtnetlink.h>
+#include <net/genetlink.h>
+
+#include "cna.h"
+
+/* Attributes */
+enum {
+ BNAD_GENL_ATTR_UNSPEC,
+ BNAD_GENL_ATTR_IOC_INFO,
+ BNAD_GENL_ATTR_IOC_ATTR,
+ __BNAD_GENL_ATTR_MAX
+};
+
+/* Effectively a single attribute */
+#define BNAD_GENL_ATTR_MAX (__BNAD_GENL_ATTR_MAX - 1)
+
+enum {
+ BNAD_GENL_VERSION = 1,
+};
+
+/* Commands/Responses */
+enum {
+ BNAD_GENL_CMD_UNSPEC,
+ BNAD_GENL_CMD_IOC_INFO,
+ BNAD_GENL_CMD_IOC_ATTR,
+ __BNAD_GENL_CMD_MAX,
+};
+
+struct bnad_genl_ioc_info {
+ int status;
+ u16 bnad_num;
+ u16 rsvd;
+ char serialnum[64];
+ char hwpath[BFA_STRING_32];
+ char adapter_hwpath[BFA_STRING_32];
+ char guid[BFA_ADAPTER_SYM_NAME_LEN*2];
+ char name[BFA_ADAPTER_SYM_NAME_LEN];
+ char port_name[BFA_ADAPTER_SYM_NAME_LEN];
+ char eth_name[BFA_ADAPTER_SYM_NAME_LEN];
+ u64 rsvd1[4];
+ mac_t mac;
+ mac_t factory_mac; /* Factory mac address */
+ mac_t current_mac; /* Currently assigned mac address */
+ enum bfa_ioc_type ioc_type;
+ u16 pvid; /* Port vlan id */
+ u16 rsvd2;
+ u32 host;
+ u32 bandwidth; /* For PF support */
+ u32 rsvd3;
+};
+
+struct bnad_genl_ioc_attr {
+ int status;
+ u16 bnad_num;
+ u16 rsvd;
+ struct bfa_ioc_attr ioc_attr;
+};
+
+extern int bnad_genl_init(void);
+extern int bnad_genl_uninit(void);
+
+#endif /* __BNAD_GENL_H__ */
--
1.7.1
^ permalink raw reply related
* [PATCH 1/2] bna: Add Debugfs Interface
From: Rasesh Mody @ 2011-05-18 4:57 UTC (permalink / raw)
To: davem, netdev; +Cc: adapter_linux_open_src_team, Rasesh Mody, Debashis Dutt
In-Reply-To: <1305694621-28023-1-git-send-email-rmody@brocade.com>
This patch adds the debugfs interface to BNA driver for collecting both
live and saved firmware traces (saved, in case of a firmware heart beat
failure).
Signed-off-by: Debashis Dutt <ddutt@brocade.com>
Signed-off-by: Rasesh Mody <rmody@brocade.com>
---
drivers/net/bna/Makefile | 3 +-
drivers/net/bna/bfa_ioc.c | 105 ++++++++++++++-
drivers/net/bna/bfa_ioc.h | 6 +
drivers/net/bna/bfi.h | 2 +
drivers/net/bna/bna_ctrl.c | 5 +-
drivers/net/bna/bnad.c | 37 +++++-
drivers/net/bna/bnad.h | 15 ++-
drivers/net/bna/bnad_debugfs.c | 302 ++++++++++++++++++++++++++++++++++++++++
8 files changed, 466 insertions(+), 9 deletions(-)
create mode 100644 drivers/net/bna/bnad_debugfs.c
diff --git a/drivers/net/bna/Makefile b/drivers/net/bna/Makefile
index a5d604d..4bb0d5d 100644
--- a/drivers/net/bna/Makefile
+++ b/drivers/net/bna/Makefile
@@ -5,7 +5,8 @@
obj-$(CONFIG_BNA) += bna.o
-bna-objs := bnad.o bnad_ethtool.o bna_ctrl.o bna_txrx.o
+bna-objs := bnad.o bnad_debugfs.o bnad_ethtool.o
+bna-objs += bna_ctrl.o bna_txrx.o
bna-objs += bfa_ioc.o bfa_ioc_ct.o bfa_cee.o cna_fwimg.o
EXTRA_CFLAGS := -Idrivers/net/bna
diff --git a/drivers/net/bna/bfa_ioc.c b/drivers/net/bna/bfa_ioc.c
index fcb9bb3..15f9dec 100644
--- a/drivers/net/bna/bfa_ioc.c
+++ b/drivers/net/bna/bfa_ioc.c
@@ -1652,6 +1652,7 @@ bfa_ioc_fail_notify(struct bfa_ioc *ioc)
{
struct list_head *qe;
struct bfa_ioc_hbfail_notify *notify;
+ int tlen;
/**
* Notify driver and common modules registered for notification.
@@ -1661,6 +1662,15 @@ bfa_ioc_fail_notify(struct bfa_ioc *ioc)
notify = (struct bfa_ioc_hbfail_notify *) qe;
notify->cbfn(notify->cbarg);
}
+
+ /* Save firmware trace if configured. */
+ if (ioc->dbg_fwsave_once) {
+ ioc->dbg_fwsave_once = false;
+ if (ioc->dbg_fwsave_len) {
+ tlen = ioc->dbg_fwsave_len;
+ bfa_nw_ioc_debug_fwtrc(ioc, ioc->dbg_fwsave, &tlen);
+ }
+ }
}
static void
@@ -1922,6 +1932,17 @@ bfa_ioc_smem_pgnum(struct bfa_ioc *ioc, u32 fmaddr)
return PSS_SMEM_PGNUM(ioc->ioc_regs.smem_pg0, fmaddr);
}
+/*
+ * Initialize memory for saving firmware trace. Driver must initialize
+ * trace memory before call bfa_ioc_enable().
+ */
+void
+bfa_nw_ioc_debug_memclaim(struct bfa_ioc *ioc, void *dbg_fwsave)
+{
+ ioc->dbg_fwsave = dbg_fwsave;
+ ioc->dbg_fwsave_len = (ioc->iocpf.auto_recover) ? BFA_DBG_FWTRC_LEN : 0;
+}
+
/**
* Register mailbox message handler function, to be called by common modules
*/
@@ -2209,13 +2230,95 @@ bfa_nw_ioc_get_mac(struct bfa_ioc *ioc)
return ioc->attr->mac;
}
+static int
+bfa_ioc_smem_read(struct bfa_ioc *ioc, void *tbuf, u32 soff, u32 sz)
+{
+ u32 pgnum, loff;
+ __be32 r32;
+ int i, len;
+ u32 *buf = tbuf;
+
+ pgnum = PSS_SMEM_PGNUM(ioc->ioc_regs.smem_pg0, soff);
+ loff = PSS_SMEM_PGOFF(soff);
+
+ /*
+ * Hold semaphore to serialize pll init and fwtrc.
+ */
+ if (!(bfa_nw_ioc_sem_get(ioc->ioc_regs.ioc_init_sem_reg)))
+ return 1;
+
+ writel(pgnum, ioc->ioc_regs.host_page_num_fn);
+
+ len = sz/sizeof(u32);
+ for (i = 0; i < len; i++) {
+ r32 = swab32(readl(ioc->ioc_regs.smem_page_start + loff));
+ buf[i] = be32_to_cpu(r32);
+ loff += sizeof(u32);
+
+ /*
+ * handle page offset wrap around
+ */
+ loff = PSS_SMEM_PGOFF(loff);
+ if (loff == 0) {
+ pgnum++;
+ writel(pgnum, ioc->ioc_regs.host_page_num_fn);
+ }
+ }
+ writel(PSS_SMEM_PGNUM(ioc->ioc_regs.smem_pg0, 0),
+ ioc->ioc_regs.host_page_num_fn);
+ /*
+ * release semaphore.
+ */
+ writel(1, ioc->ioc_regs.ioc_init_sem_reg);
+
+ return 0;
+}
+
+int
+bfa_nw_ioc_debug_fwtrc(struct bfa_ioc *ioc, void *trcdata, int *trclen)
+{
+ u32 loff = (BFI_IOC_TRC_OFF + BFA_DBG_FWTRC_LEN * (ioc->port_id));
+ int tlen, status = 0;
+
+ tlen = *trclen;
+ if (tlen > BFA_DBG_FWTRC_LEN)
+ tlen = BFA_DBG_FWTRC_LEN;
+
+ status = bfa_ioc_smem_read(ioc, trcdata, loff, tlen);
+ *trclen = tlen;
+ return status;
+}
+
+/**
+ * Retrieve saved firmware trace from a prior IOC failure.
+ */
+int
+bfa_nw_ioc_debug_fwsave(struct bfa_ioc *ioc, void *trcdata, int *trclen)
+{
+ int tlen;
+
+ if (ioc->iocpf.auto_recover)
+ ioc->dbg_fwsave_len = BFA_DBG_FWTRC_LEN;
+ else
+ return BFA_STATUS_ENOFSAVE;
+
+ tlen = *trclen;
+ if (tlen > ioc->dbg_fwsave_len)
+ tlen = ioc->dbg_fwsave_len;
+
+ memcpy(trcdata, ioc->dbg_fwsave, tlen);
+ *trclen = tlen;
+ return BFA_STATUS_OK;
+}
+
/**
* Firmware failure detected. Start recovery actions.
*/
static void
bfa_ioc_recover(struct bfa_ioc *ioc)
{
- pr_crit("Heart Beat of IOC has failed\n");
+ pr_crit("bna: Heart Beat of IOC has failed for pci funtion %u\n",
+ ioc->pcidev.pci_func);
bfa_ioc_stats(ioc, ioc_hbfails);
bfa_fsm_send_event(ioc, IOC_E_HBFAIL);
}
diff --git a/drivers/net/bna/bfa_ioc.h b/drivers/net/bna/bfa_ioc.h
index bd48abe..49739cd 100644
--- a/drivers/net/bna/bfa_ioc.h
+++ b/drivers/net/bna/bfa_ioc.h
@@ -19,6 +19,9 @@
#ifndef __BFA_IOC_H__
#define __BFA_IOC_H__
+#define BFA_DBG_FWTRC_LEN (BFI_IOC_TRC_ENTS * BFI_IOC_TRC_ENT_SZ + \
+ BFI_IOC_TRC_HDR_SZ)
+
#include "bfa_sm.h"
#include "bfi.h"
#include "cna.h"
@@ -274,6 +277,9 @@ void bfa_nw_ioc_fwver_get(struct bfa_ioc *ioc,
bool bfa_nw_ioc_fwver_cmp(struct bfa_ioc *ioc,
struct bfi_ioc_image_hdr *fwhdr);
mac_t bfa_nw_ioc_get_mac(struct bfa_ioc *ioc);
+void bfa_nw_ioc_debug_memclaim(struct bfa_ioc *ioc, void *dbg_fwsave);
+int bfa_nw_ioc_debug_fwtrc(struct bfa_ioc *ioc, void *trcdata, int *trclen);
+int bfa_nw_ioc_debug_fwsave(struct bfa_ioc *ioc, void *trcdata, int *trclen);
/*
* Timeout APIs
diff --git a/drivers/net/bna/bfi.h b/drivers/net/bna/bfi.h
index 6050379..ee73b6f 100644
--- a/drivers/net/bna/bfi.h
+++ b/drivers/net/bna/bfi.h
@@ -277,6 +277,8 @@ struct bfi_ioc_getattr_reply {
*/
#define BFI_IOC_TRC_OFF (0x4b00)
#define BFI_IOC_TRC_ENTS 256
+#define BFI_IOC_TRC_ENT_SZ 16
+#define BFI_IOC_TRC_HDR_SZ 32
#define BFI_IOC_FW_SIGNATURE (0xbfadbfad)
#define BFI_IOC_MD5SUM_SZ 4
diff --git a/drivers/net/bna/bna_ctrl.c b/drivers/net/bna/bna_ctrl.c
index 53b1416..a4833e3 100644
--- a/drivers/net/bna/bna_ctrl.c
+++ b/drivers/net/bna/bna_ctrl.c
@@ -1681,6 +1681,7 @@ bna_adv_device_init(struct bna_device *device, struct bna *bna,
device->bna = bna;
kva = res_info[BNA_RES_MEM_T_FWTRC].res_u.mem_info.mdl[0].kva;
+ bfa_nw_ioc_debug_memclaim(&device->ioc, kva);
/**
* Attach common modules (Diag, SFP, CEE, Port) and claim respective
@@ -1820,8 +1821,8 @@ bna_adv_res_req(struct bna_res_info *res_info)
/* Virtual memory for retreiving fw_trc */
res_info[BNA_RES_MEM_T_FWTRC].res_type = BNA_RES_T_MEM;
res_info[BNA_RES_MEM_T_FWTRC].res_u.mem_info.mem_type = BNA_MEM_T_KVA;
- res_info[BNA_RES_MEM_T_FWTRC].res_u.mem_info.num = 0;
- res_info[BNA_RES_MEM_T_FWTRC].res_u.mem_info.len = 0;
+ res_info[BNA_RES_MEM_T_FWTRC].res_u.mem_info.num = 1;
+ res_info[BNA_RES_MEM_T_FWTRC].res_u.mem_info.len = BFA_DBG_FWTRC_LEN;
/* DMA memory for retreiving stats */
res_info[BNA_RES_MEM_T_STATS].res_type = BNA_RES_T_MEM;
diff --git a/drivers/net/bna/bnad.c b/drivers/net/bna/bnad.c
index e588511..a997276 100644
--- a/drivers/net/bna/bnad.c
+++ b/drivers/net/bna/bnad.c
@@ -44,8 +44,12 @@ MODULE_PARM_DESC(bnad_ioc_auto_recover, "Enable / Disable auto recovery");
/*
* Global variables
*/
+u32 bna_id;
u32 bnad_rxqs_per_cq = 2;
+struct mutex bnad_list_mutex;
+LIST_HEAD(bnad_list);
+
static const u8 bnad_bcast_addr[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
/*
@@ -72,6 +76,23 @@ do { \
#define BNAD_TXRX_SYNC_MDELAY 250 /* 250 msecs */
+static void
+bnad_add_to_list(struct bnad *bnad)
+{
+ mutex_lock(&bnad_list_mutex);
+ list_add_tail(&bnad->list_entry, &bnad_list);
+ bnad->id = bna_id++;
+ mutex_unlock(&bnad_list_mutex);
+}
+
+static void
+bnad_remove_from_list(struct bnad *bnad)
+{
+ mutex_lock(&bnad_list_mutex);
+ list_del(&bnad->list_entry);
+ mutex_unlock(&bnad_list_mutex);
+}
+
/*
* Reinitialize completions in CQ, once Rx is taken down
*/
@@ -3087,6 +3108,8 @@ bnad_pci_probe(struct pci_dev *pdev,
}
bnad = netdev_priv(netdev);
+ bnad_add_to_list(bnad);
+
/*
* PCI initialization
* Output : using_dac = 1 for 64 bit DMA
@@ -3129,6 +3152,8 @@ bnad_pci_probe(struct pci_dev *pdev,
pcidev_info.device_id = bnad->pcidev->device;
pcidev_info.pci_bar_kva = bnad->bar0;
+ bnad_debugfs_init(bnad);
+
mutex_lock(&bnad->conf_mutex);
spin_lock_irqsave(&bnad->bna_lock, flags);
@@ -3169,7 +3194,7 @@ bnad_pci_probe(struct pci_dev *pdev,
/* Finally, reguister with net_device layer */
err = register_netdev(netdev);
if (err) {
- pr_err("BNA : Registering with netdev failed\n");
+ pr_err("bna: Registering with netdev failed\n");
goto disable_device;
}
@@ -3189,6 +3214,8 @@ disable_device:
bnad_res_free(bnad);
bnad_disable_msix(bnad);
pci_uninit:
+ bnad_debugfs_uninit(bnad);
+ bnad_remove_from_list(bnad);
bnad_pci_uninit(pdev);
bnad_lock_uninit(bnad);
bnad_uninit(bnad);
@@ -3226,6 +3253,8 @@ bnad_pci_remove(struct pci_dev *pdev)
bnad_res_free(bnad);
bnad_disable_msix(bnad);
+ bnad_debugfs_uninit(bnad);
+ bnad_remove_from_list(bnad);
bnad_pci_uninit(pdev);
bnad_lock_uninit(bnad);
bnad_uninit(bnad);
@@ -3255,13 +3284,14 @@ bnad_module_init(void)
{
int err;
- pr_info("Brocade 10G Ethernet driver\n");
+ pr_info("Brocade 10G Ethernet driver - version: %s\n", BNAD_VERSION);
+ mutex_init(&bnad_list_mutex);
bfa_nw_ioc_auto_recover(bnad_ioc_auto_recover);
err = pci_register_driver(&bnad_pci_driver);
if (err < 0) {
- pr_err("bna : PCI registration failed in module init "
+ pr_err("bna: PCI registration failed in module init "
"(%d)\n", err);
return err;
}
@@ -3273,6 +3303,7 @@ static void __exit
bnad_module_exit(void)
{
pci_unregister_driver(&bnad_pci_driver);
+ mutex_destroy(&bnad_list_mutex);
if (bfi_fw)
release_firmware(bfi_fw);
diff --git a/drivers/net/bna/bnad.h b/drivers/net/bna/bnad.h
index ccdabad..2c1f283 100644
--- a/drivers/net/bna/bnad.h
+++ b/drivers/net/bna/bnad.h
@@ -279,13 +279,20 @@ struct bnad {
char adapter_name[BNAD_NAME_LEN];
char port_name[BNAD_NAME_LEN];
char mbox_irq_name[BNAD_NAME_LEN];
+
+ int id;
+ struct list_head list_entry;
+ struct dentry *port_debugfs_root;
+ struct dentry *bnad_dentry_files[2];
};
/*
* EXTERN VARIABLES
*/
-extern struct firmware *bfi_fw;
-extern u32 bnad_rxqs_per_cq;
+extern struct firmware *bfi_fw;
+extern struct mutex bnad_list_mutex;
+extern struct list_head bnad_list;
+extern u32 bnad_rxqs_per_cq;
/*
* EXTERN PROTOTYPES
@@ -306,6 +313,10 @@ extern void bnad_cleanup_rx(struct bnad *bnad, uint rx_id);
/* Timer start/stop protos */
extern void bnad_dim_timer_start(struct bnad *bnad);
+/* Debugfs */
+extern void bnad_debugfs_init(struct bnad *bnad);
+extern void bnad_debugfs_uninit(struct bnad *bnad);
+
/* Statistics */
extern void bnad_netdev_qstats_fill(struct bnad *bnad,
struct rtnl_link_stats64 *stats);
diff --git a/drivers/net/bna/bnad_debugfs.c b/drivers/net/bna/bnad_debugfs.c
new file mode 100644
index 0000000..4351ca5
--- /dev/null
+++ b/drivers/net/bna/bnad_debugfs.c
@@ -0,0 +1,302 @@
+/*
+ * Linux network driver for Brocade Converged Network Adapter.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License (GPL) Version 2 as
+ * published by the Free Software Foundation
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ */
+/*
+ * Copyright (c) 2005-2011 Brocade Communications Systems, Inc.
+ * All rights reserved
+ * www.brocade.com
+ */
+
+/*
+ * BNAD debufs interface
+ *
+ * To access the interface, debugfs file system should be mounted
+ * if not already mounted using:
+ * mount -t debugfs none /sys/kernel/debug
+ *
+ * BNAD Hierarchy:
+ * - bnad/pci_dev:<pci_name>
+ * where the pci_name corresponds to the one under
+ * /sys/bus/pci/drivers/bnad
+ *
+ * Debugging service available per pci_dev:
+ * fwtrc: To collect current firmware trace.
+ * fwsave: To collect last saved fw trace as a result of firmware crash.
+ */
+#include <linux/debugfs.h>
+
+#include "bnad.h"
+
+struct bnad_debug_info {
+ char *debug_buffer;
+ int buffer_len;
+};
+
+struct bnad_debugfs_entry {
+ const char *name;
+ mode_t mode;
+ const struct file_operations *fops;
+};
+
+static int
+bnad_debugfs_open_fwtrc(struct inode *inode, struct file *file)
+{
+ struct bnad *bnad = inode->i_private;
+ struct bnad_debug_info *fw_debug;
+ unsigned long flags;
+ int rc;
+
+ fw_debug = kzalloc(sizeof(struct bnad_debug_info), GFP_KERNEL);
+ if (!fw_debug)
+ return -ENOMEM;
+
+ fw_debug->buffer_len = BFA_DBG_FWTRC_LEN;
+
+ fw_debug->debug_buffer = kzalloc(fw_debug->buffer_len, GFP_KERNEL);
+ if (!fw_debug->debug_buffer) {
+ kfree(fw_debug);
+ fw_debug = NULL;
+ pr_warn("bnad[%d]: Failed to allocate fwtrc buffer\n",
+ bnad->id);
+ return -ENOMEM;
+ }
+
+ spin_lock_irqsave(&bnad->bna_lock, flags);
+ rc = bfa_nw_ioc_debug_fwtrc(&bnad->bna.device.ioc,
+ fw_debug->debug_buffer,
+ &fw_debug->buffer_len);
+ spin_unlock_irqrestore(&bnad->bna_lock, flags);
+ if (rc != BFA_STATUS_OK) {
+ kfree(fw_debug->debug_buffer);
+ fw_debug->debug_buffer = NULL;
+ kfree(fw_debug);
+ fw_debug = NULL;
+ pr_warn("bnad[%d]: Failed to collect fwtrc\n", bnad->id);
+ return -ENOMEM;
+ }
+
+ file->private_data = fw_debug;
+
+ return 0;
+}
+
+static int
+bnad_debugfs_open_fwsave(struct inode *inode, struct file *file)
+{
+ struct bnad *bnad = inode->i_private;
+ struct bnad_debug_info *fw_debug;
+ unsigned long flags;
+ int rc;
+
+ fw_debug = kzalloc(sizeof(struct bnad_debug_info), GFP_KERNEL);
+ if (!fw_debug)
+ return -ENOMEM;
+
+ fw_debug->buffer_len = BFA_DBG_FWTRC_LEN;
+
+ fw_debug->debug_buffer = kzalloc(fw_debug->buffer_len, GFP_KERNEL);
+ if (!fw_debug->debug_buffer) {
+ kfree(fw_debug);
+ fw_debug = NULL;
+ pr_warn("bnad[%d]: Failed to allocate fwsave buffer\n",
+ bnad->id);
+ return -ENOMEM;
+ }
+
+ spin_lock_irqsave(&bnad->bna_lock, flags);
+ rc = bfa_nw_ioc_debug_fwsave(&bnad->bna.device.ioc,
+ fw_debug->debug_buffer,
+ &fw_debug->buffer_len);
+ spin_unlock_irqrestore(&bnad->bna_lock, flags);
+ if (rc != BFA_STATUS_OK && rc != BFA_STATUS_ENOFSAVE) {
+ kfree(fw_debug->debug_buffer);
+ fw_debug->debug_buffer = NULL;
+ kfree(fw_debug);
+ fw_debug = NULL;
+ pr_warn("bnad[%d]: Failed to collect fwsave\n", bnad->id);
+ return -ENOMEM;
+ }
+
+ file->private_data = fw_debug;
+
+ return 0;
+}
+
+/* Changes the current file position */
+static loff_t
+bnad_debugfs_lseek(struct file *file, loff_t offset, int orig)
+{
+ loff_t pos = file->f_pos;
+ struct bnad_debug_info *debug = file->private_data;
+
+ if (!debug)
+ return -EINVAL;
+
+ switch (orig) {
+ case 0:
+ file->f_pos = offset;
+ break;
+ case 1:
+ file->f_pos += offset;
+ break;
+ case 2:
+ file->f_pos = debug->buffer_len - offset;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if (file->f_pos < 0 || file->f_pos > debug->buffer_len) {
+ file->f_pos = pos;
+ return -EINVAL;
+ }
+
+ return file->f_pos;
+}
+
+static ssize_t
+bnad_debugfs_read(struct file *file, char __user *buf,
+ size_t nbytes, loff_t *pos)
+{
+ struct bnad_debug_info *debug = file->private_data;
+
+ if (!debug || !debug->debug_buffer)
+ return 0;
+
+ return simple_read_from_buffer(buf, nbytes, pos,
+ debug->debug_buffer, debug->buffer_len);
+}
+
+static int
+bnad_debugfs_release_fwtrc(struct inode *inode, struct file *file)
+{
+ struct bnad_debug_info *fw_debug = file->private_data;
+
+ if (!fw_debug)
+ return 0;
+
+ kfree(fw_debug->debug_buffer);
+
+ file->private_data = NULL;
+ kfree(fw_debug);
+ fw_debug = NULL;
+ return 0;
+}
+
+static const struct file_operations bnad_debugfs_op_fwtrc = {
+ .owner = THIS_MODULE,
+ .open = bnad_debugfs_open_fwtrc,
+ .llseek = bnad_debugfs_lseek,
+ .read = bnad_debugfs_read,
+ .release = bnad_debugfs_release_fwtrc,
+};
+
+static const struct file_operations bnad_debugfs_op_fwsave = {
+ .owner = THIS_MODULE,
+ .open = bnad_debugfs_open_fwsave,
+ .llseek = bnad_debugfs_lseek,
+ .read = bnad_debugfs_read,
+ .release = bnad_debugfs_release_fwtrc,
+};
+
+static const struct bnad_debugfs_entry bnad_debugfs_files[] = {
+ { "fwtrc", S_IFREG|S_IRUGO, &bnad_debugfs_op_fwtrc, },
+ { "fwsave", S_IFREG|S_IRUGO, &bnad_debugfs_op_fwsave, },
+};
+
+/* Global varibales */
+static struct dentry *bnad_debugfs_root;
+static atomic_t bnad_debugfs_port_count;
+
+/* Initialize debugfs interface for BNAD */
+void
+bnad_debugfs_init(struct bnad *bnad)
+{
+ const struct bnad_debugfs_entry *file;
+ char name[64];
+ int i;
+
+ /* Setup the BNAD debugfs root directory*/
+ mutex_lock(&bnad_list_mutex);
+ if (!bnad_debugfs_root) {
+ bnad_debugfs_root = debugfs_create_dir("bnad", NULL);
+ atomic_set(&bnad_debugfs_port_count, 0);
+ if (!bnad_debugfs_root) {
+ mutex_unlock(&bnad_list_mutex);
+ pr_warn("BNAD debugfs root dir creation failed\n");
+ return;
+ }
+ }
+ mutex_unlock(&bnad_list_mutex);
+
+ /* Setup the pci_dev debugfs directory for the port */
+ snprintf(name, sizeof(name), "pci_dev:%s", pci_name(bnad->pcidev));
+ if (!bnad->port_debugfs_root) {
+ bnad->port_debugfs_root =
+ debugfs_create_dir(name, bnad_debugfs_root);
+ if (!bnad->port_debugfs_root) {
+ pr_warn("BNAD pci_dev:%s root dir creation failed\n",
+ pci_name(bnad->pcidev));
+ return;
+ }
+
+ atomic_inc(&bnad_debugfs_port_count);
+
+ for (i = 0; i < ARRAY_SIZE(bnad_debugfs_files); i++) {
+ file = &bnad_debugfs_files[i];
+ bnad->bnad_dentry_files[i] =
+ debugfs_create_file(file->name,
+ file->mode,
+ bnad->port_debugfs_root,
+ bnad,
+ file->fops);
+ if (!bnad->bnad_dentry_files[i]) {
+ pr_warn(
+ "BNAD pci_dev:%s: create %s entry \
+failed\n", pci_name(bnad->pcidev), file->name);
+ return;
+ }
+ }
+ }
+
+ pr_info("bnad[%d]: Initialized debugfs interface\n", bnad->id);
+}
+
+/* Uninitialize debugfs interface for BNAD */
+void
+bnad_debugfs_uninit(struct bnad *bnad)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(bnad_debugfs_files); i++) {
+ if (bnad->bnad_dentry_files[i]) {
+ debugfs_remove(bnad->bnad_dentry_files[i]);
+ bnad->bnad_dentry_files[i] = NULL;
+ }
+ }
+
+ /* Remove the pci_dev debugfs directory for the port */
+ if (bnad->port_debugfs_root) {
+ debugfs_remove(bnad->port_debugfs_root);
+ bnad->port_debugfs_root = NULL;
+ atomic_dec(&bnad_debugfs_port_count);
+ }
+
+ /* Remove the BNAD debugfs root directory */
+ mutex_lock(&bnad_list_mutex);
+ if (atomic_read(&bnad_debugfs_port_count) == 0) {
+ debugfs_remove(bnad_debugfs_root);
+ bnad_debugfs_root = NULL;
+ }
+ mutex_unlock(&bnad_list_mutex);
+ pr_info("bnad[%d]: Uninitialized debugfs interface\n", bnad->id);
+}
--
1.7.1
^ permalink raw reply related
* [PATCH 0/2] bna: Add Debugfs & Generic Netlink Interfaces to BNA Driver
From: Rasesh Mody @ 2011-05-18 4:56 UTC (permalink / raw)
To: davem, netdev; +Cc: adapter_linux_open_src_team, Rasesh Mody
This patch set adds debugfs and generic netlink interfaces to BNA driver for
debugging and managing the Brocade adapter.
Rasesh Mody (2):
bna: Add Debugfs Interface
bna: Add Generic Netlink Interface
drivers/net/bna/Makefile | 3 +-
drivers/net/bna/bfa_defs.h | 18 ++
drivers/net/bna/bfa_ioc.c | 113 +++++++++++++-
drivers/net/bna/bfa_ioc.h | 7 +
drivers/net/bna/bfi.h | 2 +
drivers/net/bna/bna_ctrl.c | 5 +-
drivers/net/bna/bnad.c | 48 +++++-
drivers/net/bna/bnad.h | 16 ++-
drivers/net/bna/bnad_debugfs.c | 302 +++++++++++++++++++++++++++++++++++
drivers/net/bna/bnad_genl.c | 345 ++++++++++++++++++++++++++++++++++++++++
drivers/net/bna/bnad_genl.h | 87 ++++++++++
11 files changed, 931 insertions(+), 15 deletions(-)
create mode 100644 drivers/net/bna/bnad_debugfs.c
create mode 100644 drivers/net/bna/bnad_genl.c
create mode 100644 drivers/net/bna/bnad_genl.h
^ permalink raw reply
* Re: [PATCH net-next-2.6] net: add rcu protection to netdev->ifalias
From: Eric Dumazet @ 2011-05-18 4:35 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, kaber, netdev, remi.denis-courmont
In-Reply-To: <1305671956.6741.25.camel@edumazet-laptop>
Le mercredi 18 mai 2011 à 00:39 +0200, Eric Dumazet a écrit :
> I dont know, if it happens to be too hard, we'll just stick again rtnl
> for "ip link show" ;)
>
>
I believe I'll take this path, its a bit too hard for the moment, and
need several preliminary steps :
It is making sense trying to get rid of rtnl_trylock() hack with more
fine grained locks, and thus lower pressure on RTNL.
Some synchronize_rcu() calls are done whith RTNL held : All processes
hitting rtnl_trylock() have to enter a busy loop, restarting syscall as
long as the RTNL owner is blocked in synchronize_rcu().
^ permalink raw reply
* [PATCH] Check the value of doi before referencing it
From: Huzaifa Sidhpurwala @ 2011-05-18 4:29 UTC (permalink / raw)
To: netdev; +Cc: kaber, yoshfuji, jmorris, pekkas, kuznet, davem,
Huzaifa Sidhpurwala
Value of doi is not checked before referencing it.
Though this does not cause any null pointer dereference since
all the callers of cipso_v4_doi_add check the value of doi
before calling the function, but it would be a good programming
practice to do so anyways :)
Signed-off-by: Huzaifa Sidhpurwala <huzaifas@redhat.com>
---
net/ipv4/cipso_ipv4.c | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
index a0af7ea..7adc4ea 100644
--- a/net/ipv4/cipso_ipv4.c
+++ b/net/ipv4/cipso_ipv4.c
@@ -473,10 +473,13 @@ int cipso_v4_doi_add(struct cipso_v4_doi *doi_def,
u32 doi_type;
struct audit_buffer *audit_buf;
- doi = doi_def->doi;
- doi_type = doi_def->type;
+ if (doi_def) {
+ doi = doi_def->doi;
+ doi_type = doi_def->type;
+ } else
+ goto doi_add_return;
- if (doi_def == NULL || doi_def->doi == CIPSO_V4_DOI_UNKNOWN)
+ if (doi_def->doi == CIPSO_V4_DOI_UNKNOWN)
goto doi_add_return;
for (iter = 0; iter < CIPSO_V4_TAG_MAXCNT; iter++) {
switch (doi_def->tags[iter]) {
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] e100: Fix race condition in e100_down() while testing
From: Prasanna Panchamukhi @ 2011-05-18 3:07 UTC (permalink / raw)
Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
bruce.w.allan@intel.com, jesse.brandeburg@intel.com
In-Reply-To: <1305670825-13603-1-git-send-email-prasanna.panchamukhi@riverbed.com>
Please ignore this, sending and updated one..
On 05/17/2011 03:20 PM, prasanna.panchamukhi@riverbed.com wrote:
> There is a race condition between e100_down() and e100_diag_test().
> During device testing e100_diag_test() ends up calling e100_up()/e100_down()
> even while the driver is already in e100_down().
> This patch fixes the above race condition by changing
> e100_up()/e100_down() to dev_open() and dev_close().
> Also fixes the race between e100_open() and e100_diag_test().
>
> Signed-off-by: Prasanna S. Panchamukh<prasanna.panchamukhi@riverbed.com>
> ---
> drivers/net/e100.c | 21 +++++++++++++++++----
> 1 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/e100.c b/drivers/net/e100.c
> index b0aa9e6..abbf229 100644
> --- a/drivers/net/e100.c
> +++ b/drivers/net/e100.c
> @@ -425,6 +425,10 @@ enum cb_command {
> cb_el = 0x8000,
> };
>
> +enum e100_state_t {
> + __E100_TESTING
> +};
> +
> struct rfd {
> __le16 status;
> __le16 command;
> @@ -623,6 +627,7 @@ struct nic {
> __le16 eeprom[256];
> spinlock_t mdio_lock;
> const struct firmware *fw;
> + unsigned long state;
> };
>
> static inline void e100_write_flush(struct nic *nic)
> @@ -2570,8 +2575,10 @@ static void e100_diag_test(struct net_device *netdev,
> {
> struct ethtool_cmd cmd;
> struct nic *nic = netdev_priv(netdev);
> + int if_running = netif_running(netdev);
> int i, err;
>
> + set_bit(__E100_TESTING,&nic->state);
> memset(data, 0, E100_TEST_LEN * sizeof(u64));
> data[0] = !mii_link_ok(&nic->mii);
> data[1] = e100_eeprom_load(nic);
> @@ -2580,8 +2587,9 @@ static void e100_diag_test(struct net_device *netdev,
> /* save speed, duplex& autoneg settings */
> err = mii_ethtool_gset(&nic->mii,&cmd);
>
> - if (netif_running(netdev))
> - e100_down(nic);
> + if (if_running)
> + /* indicate we're in test mode */
> + dev_open(netdev);
> data[2] = e100_self_test(nic);
> data[3] = e100_loopback_test(nic, lb_mac);
> data[4] = e100_loopback_test(nic, lb_phy);
> @@ -2589,12 +2597,13 @@ static void e100_diag_test(struct net_device *netdev,
> /* restore speed, duplex& autoneg settings */
> err = mii_ethtool_sset(&nic->mii,&cmd);
>
> - if (netif_running(netdev))
> - e100_up(nic);
> + if (if_running)
> + dev_open(netdev);
> }
> for (i = 0; i< E100_TEST_LEN; i++)
> test->flags |= data[i] ? ETH_TEST_FL_FAILED : 0;
>
> + clear_bit(__E100_TESTING,&nic->state);
> msleep_interruptible(4 * 1000);
> }
>
> @@ -2724,6 +2733,10 @@ static int e100_open(struct net_device *netdev)
> struct nic *nic = netdev_priv(netdev);
> int err = 0;
>
> + /* disallow open during testing */
> + if (test_bit(__E100_TESTING,&nic->state))
> + return -EBUSY;
> +
> netif_carrier_off(netdev);
> if ((err = e100_up(nic)))
> netif_err(nic, ifup, nic->netdev, "Cannot open interface, aborting\n");
------------------------------------------------------------------------------
What Every C/C++ and Fortran developer Should Know!
Read this article and learn how Intel has extended the reach of its
next-generation tools to help Windows* and Linux* C/C++ and Fortran
developers boost performance applications - including clusters.
http://p.sf.net/sfu/intel-dev2devmay
_______________________________________________
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-next-2.6] sfc: Replace enum efx_fc_type with a 'bitwise' type
From: Jeff Garzik @ 2011-05-18 2:59 UTC (permalink / raw)
To: David Miller; +Cc: bhutchings, netdev
In-Reply-To: <20110517.222302.158419141035335293.davem@davemloft.net>
On 05/17/2011 10:23 PM, David Miller wrote:
> From: Jeff Garzik<jeff@garzik.org>
>
>> On 05/17/2011 05:14 PM, David Miller wrote:
>>> Accept that the compiler currently doesn't want to allow enums to be
>>> used as bit-masks, don't paper around it.
>>
>> This practice is all over the kernel.
>>
>> It's a bit silly to make such a big deal, because of that.
>
> "We do something stupid everywhere" is never a good
> argument for anything.
grepping across the kernel, it is clearly a useful shorthand technique
for defining typed, named constants, including bitmasks, used widely
across multiple subsystems, drivers and include/linux.
Given that this technique has passed review time and again, and that
kernel hackers -do- find it useful for valid reasons, I guess the only
remaining defense is to call something "stupid."
Jeff
^ permalink raw reply
* Re: [PATCH net-next-2.6] sfc: Replace enum efx_fc_type with a 'bitwise' type
From: David Miller @ 2011-05-18 2:23 UTC (permalink / raw)
To: jeff; +Cc: bhutchings, netdev
In-Reply-To: <4DD3285D.6010506@garzik.org>
From: Jeff Garzik <jeff@garzik.org>
Date: Tue, 17 May 2011 22:01:01 -0400
> On 05/17/2011 05:14 PM, David Miller wrote:
>> Accept that the compiler currently doesn't want to allow enums to be
>> used as bit-masks, don't paper around it.
>
> This practice is all over the kernel.
>
> It's a bit silly to make such a big deal, because of that.
"We do something stupid everywhere" is never a good
argument for anything.
^ permalink raw reply
* Re: [PATCH net-next-2.6] sfc: Replace enum efx_fc_type with a 'bitwise' type
From: Jeff Garzik @ 2011-05-18 2:01 UTC (permalink / raw)
To: David Miller; +Cc: bhutchings, netdev
In-Reply-To: <20110517.171412.1017451005914294196.davem@davemloft.net>
On 05/17/2011 05:14 PM, David Miller wrote:
> Accept that the compiler currently doesn't want to allow enums to be
> used as bit-masks, don't paper around it.
This practice is all over the kernel.
It's a bit silly to make such a big deal, because of that.
Jeff
^ permalink raw reply
* Re: bonding flaps between member interfaces
From: Jay Vosburgh @ 2011-05-18 1:22 UTC (permalink / raw)
To: Patrick Schaaf; +Cc: netdev
In-Reply-To: <1305638854.6044.223.camel@lat1>
Patrick Schaaf <netdev@bof.de> wrote:
>Dear netdev,
>
>I'm experiencing a regression with bonding. Bugzilla and cursory
>searching of the list did not immediately show up anything that seems
>related, so here's the report:
>
>Short summary: bonding flips between members every second
I have reproduced the problem on a 2.6.38-rc5-ish kernel.
The described configuration is enslaving two VLAN interfaces; I
also tried enslaving eth0/eth1 directly and stacking the VLAN atop
bonding. That doesn't work either. I don't get any errors, and bonding
says the slaves are up, but ping through the VLAN fails. Ping over the
non-VLAN (directly on bond0) works ok.
I'll give it some bisect action and report back.
-J
>bonding in active-backup mode with ARP monitoring
>two members in the bond, both being VLAN interfaces on top of two
>separate ethernet interfaces
>bnx2 ethernet driver, but saw the same behaviour with a tigon box
>concrete settings:
>BONDING_MODULE_OPTS="mode=active-backup primary=eth0.24 arp_interval=250
>arp_ip_target=192.168.x.x"
>See below for a /proc/net/bonding/bond24 output reflecing the
>configuration.
>
>This setup I have in production on 2.6.36.2, and it works fine.
>It also works fine, tested today, with 2.6.36,4 and 2.6.37.6
>
>Starting with 2.6.38 (2.6.38.6 tested today), and still happening with
>2.6.39-rc7, I experience problems. While I can still work over the
>interface, it is flipping once per second between the two member
>interfaces. There is no indication of the underlying interface going
>up/down, but bonding seems to think so.
>
>See below an excerpt of the kernel log for two back-and-forth flapping
>cycles.
>
>In /proc/net/bonding/bond24, I see the failure counter of the configured
>primary interface counting up with each flap. The counter of the non
>primary interface does not move. When I switch the primary interface by
>echoing to /sys, the behaviour of the counters flips: always the
>configured primary has the counter going up.
>
>best regards
> Patrick
>
>Here is /proc/net/bonding/bond24 while running on 2.6.37.6, to show the
>concrete configuration from this POV. Everything looks the same with the
>failing kernels, except for the noted behaviour of the Failure Counts.
>
>Ethernet Channel Bonding Driver: v3.7.0 (June 2, 2010)
>
>Bonding Mode: fault-tolerance (active-backup)
>Primary Slave: eth0.24 (primary_reselect always)
>Currently Active Slave: eth0.24
>MII Status: up
>MII Polling Interval (ms): 0
>Up Delay (ms): 0
>Down Delay (ms): 0
>ARP Polling Interval (ms): 250
>ARP IP target/s (n.n.n.n form): 192.168.x.x
>
>Slave Interface: eth0.24
>MII Status: up
>Speed: 1000 Mbps
>Duplex: full
>Link Failure Count: 0
>Permanent HW addr: d4:85:64:ca:1c:12
>Slave queue ID: 0
>
>Slave Interface: eth1.24
>MII Status: up
>Speed: 1000 Mbps
>Duplex: full
>Link Failure Count: 0
>Permanent HW addr: d4:85:64:ca:1c:14
>Slave queue ID: 0
>
>Here is kernel log output for two flapping cycles (booted kernel was
>2.6.39-rc7):
>
>May 17 14:58:22 myserver kernel: [ 1016.629155] bonding: bond24: link
>status definitely down for interface eth0.24, disabling it
>May 17 14:58:22 myserver kernel: [ 1016.629159] bonding: bond24: making
>interface eth1.24 the new active one.
>May 17 14:58:22 myserver kernel: [ 1016.629162] device eth0.24 left
>promiscuous mode
>May 17 14:58:22 myserver kernel: [ 1016.629164] device eth0 left
>promiscuous mode
>May 17 14:58:22 myserver kernel: [ 1016.629191] device eth1.24 entered
>promiscuous mode
>May 17 14:58:22 myserver kernel: [ 1016.629193] device eth1 entered
>promiscuous mode
>May 17 14:58:22 myserver kernel: [ 1016.878596] bonding: bond24: link
>status definitely up for interface eth0.24.
>May 17 14:58:22 myserver kernel: [ 1016.878600] bonding: bond24: making
>interface eth0.24 the new active one.
>May 17 14:58:22 myserver kernel: [ 1016.878603] device eth1.24 left
>promiscuous mode
>May 17 14:58:22 myserver kernel: [ 1016.878605] device eth1 left
>promiscuous mode
>May 17 14:58:22 myserver kernel: [ 1016.878631] device eth0.24 entered
>promiscuous mode
>May 17 14:58:22 myserver kernel: [ 1016.878633] device eth0 entered
>promiscuous mode
>May 17 14:58:23 myserver kernel: [ 1017.626919] bonding: bond24: link
>status definitely down for interface eth0.24, disabling it
>May 17 14:58:23 myserver kernel: [ 1017.626923] bonding: bond24: making
>interface eth1.24 the new active one.
>May 17 14:58:23 myserver kernel: [ 1017.626926] device eth0.24 left
>promiscuous mode
>May 17 14:58:23 myserver kernel: [ 1017.626928] device eth0 left
>promiscuous mode
>May 17 14:58:23 myserver kernel: [ 1017.626955] device eth1.24 entered
>promiscuous mode
>May 17 14:58:23 myserver kernel: [ 1017.626957] device eth1 entered
>promiscuous mode
>May 17 14:58:23 myserver kernel: [ 1017.876359] bonding: bond24: link
>status definitely up for interface eth0.24.
>May 17 14:58:23 myserver kernel: [ 1017.876363] bonding: bond24: making
>interface eth0.24 the new active one.
>May 17 14:58:23 myserver kernel: [ 1017.876366] device eth1.24 left
>promiscuous mode
>May 17 14:58:23 myserver kernel: [ 1017.876368] device eth1 left
>promiscuous mode
>May 17 14:58:23 myserver kernel: [ 1017.876394] device eth0.24 entered
>promiscuous mode
>May 17 14:58:23 myserver kernel: [ 1017.876396] device eth0 entered
>promiscuous mode
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply
* Re: [PATCH] IPv6 transmit hashing for bonding driver
From: Jay Vosburgh @ 2011-05-18 0:57 UTC (permalink / raw)
To: John; +Cc: netdev
In-Reply-To: <4DD30AF2.1090707@8192.net>
John <linux@8192.net> wrote:
>Currently the "bonding" driver does not support load balancing outgoing
>traffic in LACP mode for IPv6 traffic. IPv4 (and TCP over IPv4) are
>currently supported; this patch adds transmit hashing for IPv6 (and TCP
>over IPv6), bringing IPv6 up to par with IPv4 support in the bonding
>driver.
>
>The algorithm chosen (xor'ing the bottom three quads and then xor'ing that
>down into the bottom byte) was chosen after testing almost 400,000 unique
>IPv6 addresses harvested from server logs. This algorithm had the most
>even distribution for both big- and little-endian architectures while
>still using few instructions.
>
>This patch also adds missing configuration information the MODULE_PARM_DESC.
>
>Patch has been tested on various machines and performs as expected. Thanks
>to Stephen Hemminger and Andy Gospodarek for advice and guidance.
This looks reasonable at first glance, with a few comments
below. You'll need to supply a Signed-Off-By at some point.
It would also be useful to include an update bonding.txt to
describe the IPv6 algorithm; I'd word that something like the following
(filling in the missing bits) for the layer3+4 section, applying similar
changes to the layer2+3 section:
--- net-next-2.6/Documentation/networking/bonding.txt 2011-05-09 17:53:03.000000000 -0700
+++ net-next-2.6/Documentation/networking/bonding.txt.new 2011-05-17 17:53:46.000000000 -0700
@@ -733,21 +733,26 @@
slaves, although a single connection will not span
multiple slaves.
- The formula for unfragmented TCP and UDP packets is
+ The formula for unfragmented IPv4 TCP and UDP packets is
((source port XOR dest port) XOR
((source IP XOR dest IP) AND 0xffff)
modulo slave count
- For fragmented TCP or UDP packets and all other IP
+ The formula for unfragmented IPv6 TCP and UDP packets is
+
+ [ your formula here ]
+
+ For fragmented TCP or UDP packets and all other IP or IPv6
protocol traffic, the source and destination port
- information is omitted. For non-IP traffic, the
+ information is omitted. For non-IP/IPv6 traffic, the
formula is the same as for the layer2 transmit hash
policy.
- This policy is intended to mimic the behavior of
- certain switches, notably Cisco switches with PFC2 as
- well as some Foundry and IBM products.
+ The IPv4 behavior is intended to mimic the behavior of
+ certain switches, notably Cisco switches with PFC2 as well
+ as some Foundry and IBM products. The IPv6 behavior was
+ determined by [ your rationale here ].
This algorithm is not fully 802.3ad compliant. A
single TCP or UDP conversation containing both
>John
>
>--- drivers/net/bonding/bond_main.c.orig 2011-04-18 17:23:09.202894000 -0700
>+++ drivers/net/bonding/bond_main.c 2011-04-19 18:12:30.287929000 -0700
>@@ -152,7 +152,7 @@
> MODULE_PARM_DESC(ad_select, "803.ad aggregation selection logic: stable (0, default), bandwidth (1), count (2)");
> module_param(xmit_hash_policy, charp, 0);
> MODULE_PARM_DESC(xmit_hash_policy, "XOR hashing method: 0 for layer 2 (default)"
>- ", 1 for layer 3+4");
>+ ", 1 for layer 3+4, 2 for layer 2+3");
> module_param(arp_interval, int, 0);
> MODULE_PARM_DESC(arp_interval, "arp interval in milliseconds");
> module_param_array(arp_ip_target, charp, NULL, 0);
>@@ -3720,11 +3720,20 @@
> static int bond_xmit_hash_policy_l23(struct sk_buff *skb, int count)
> {
> struct ethhdr *data = (struct ethhdr *)skb->data;
>- struct iphdr *iph = ip_hdr(skb);
>
> if (skb->protocol == htons(ETH_P_IP)) {
>+ struct iphdr *iph = ip_hdr(skb);
> return ((ntohl(iph->saddr ^ iph->daddr) & 0xffff) ^
> (data->h_dest[5] ^ data->h_source[5])) % count;
>+ } else if (skb->protocol == htons(ETH_P_IPV6)) {
>+ struct ipv6hdr *ipv6h = ipv6_hdr(skb);
>+ u32 v6hash = (
>+ (ipv6h->saddr.s6_addr32[1] ^ ipv6h->daddr.s6_addr32[1]) ^
>+ (ipv6h->saddr.s6_addr32[2] ^ ipv6h->daddr.s6_addr32[2]) ^
>+ (ipv6h->saddr.s6_addr32[3] ^ ipv6h->daddr.s6_addr32[3])
>+ );
Style nit: I don't believe the outermost parentheses are
necessary. Since you do this twice, perhaps make a small inline
function to handle it.
>+ v6hash = (v6hash >> 16) ^ (v6hash >> 8) ^ v6hash;
>+ return (v6hash ^ data->h_dest[5] ^ data->h_source[5]) % count;
> }
>
> return (data->h_dest[5] ^ data->h_source[5]) % count;
>@@ -3738,11 +3747,11 @@
> static int bond_xmit_hash_policy_l34(struct sk_buff *skb, int count)
> {
> struct ethhdr *data = (struct ethhdr *)skb->data;
>- struct iphdr *iph = ip_hdr(skb);
>- __be16 *layer4hdr = (__be16 *)((u32 *)iph + iph->ihl);
>- int layer4_xor = 0;
>+ u32 layer4_xor = 0;
>
> if (skb->protocol == htons(ETH_P_IP)) {
>+ struct iphdr *iph = ip_hdr(skb);
>+ __be16 *layer4hdr = (__be16 *)((u32 *)iph + iph->ihl);
> if (!(iph->frag_off & htons(IP_MF|IP_OFFSET)) &&
> (iph->protocol == IPPROTO_TCP ||
> iph->protocol == IPPROTO_UDP)) {
>@@ -3750,7 +3759,18 @@
> }
> return (layer4_xor ^
> ((ntohl(iph->saddr ^ iph->daddr)) & 0xffff)) % count;
>-
>+ } else if (skb->protocol == htons(ETH_P_IPV6)) {
>+ struct ipv6hdr *ipv6h = ipv6_hdr(skb);
>+ __be16 *layer4hdrv6 = (__be16 *)((u8 *)ipv6h + sizeof(*ipv6h));
>+ if (ipv6h->nexthdr == IPPROTO_TCP || ipv6h->nexthdr == IPPROTO_UDP) {
For fragmented datagrams, the above will keep all fragments
together, which is good, but are there other header types that should be
skipped over to find the UDP/TCP header for hashing purposes?
>+ layer4_xor = (*layer4hdrv6 ^ *(layer4hdrv6 + 1));
>+ }
>+ layer4_xor ^= (
>+ (ipv6h->saddr.s6_addr32[1] ^ ipv6h->daddr.s6_addr32[1]) ^
>+ (ipv6h->saddr.s6_addr32[2] ^ ipv6h->daddr.s6_addr32[2]) ^
>+ (ipv6h->saddr.s6_addr32[3] ^ ipv6h->daddr.s6_addr32[3])
>+ );
Parentheses / maybe inline again.
>+ return ((layer4_xor >> 16) ^ (layer4_xor >> 8) ^ layer4_xor) % count;
> }
>
> return (data->h_dest[5] ^ data->h_source[5]) % count;
-J
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply
* Re: [PATCH 09/18] virtio: use avail_event index
From: Rusty Russell @ 2011-05-18 0:19 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Krishna Kumar, Carsten Otte, lguest-uLR06cmDAlY/bJ5BZ2RsiQ,
Shirley Ma, kvm-u79uwXL29TY76Z2rM5mHXA,
linux-s390-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
habanero-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, Heiko Carstens,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
virtualization-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
steved-r/Jw6+rmf7HQT0dZR+AlfA, Christian Borntraeger,
Tom Lendacky, Martin Schwidefsky, linux390-tA70FqPdS9bQT0dZR+AlfA
In-Reply-To: <20110517061031.GC26989-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Tue, 17 May 2011 09:10:31 +0300, "Michael S. Tsirkin" <mst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> Well one can imagine a driver doing:
>
> while (virtqueue_get_buf()) {
> virtqueue_add_buf()
> }
> virtqueue_kick()
>
> which looks sensible (batch kicks) but might
> process any number of bufs between kicks.
No, we currently only expose the buffers in the kick, so it can only
fill the ring doing that.
We could change that (and maybe that's worth looking at)...
> If we look at drivers closely enough, I think none
> of them do the equivalent of the above, but not 100% sure.
I'm pretty sure we don't have this kind of 'echo' driver yet. Drivers
tend to take OS requests and queue them. The only one which does
anything even partially sophisticated is the net driver...
Thanks,
Rusty.
^ permalink raw reply
* [PATCH] e100: Don't enable interrupts while exiting polling
From: prasanna.panchamukhi @ 2011-05-18 0:16 UTC (permalink / raw)
To: bruce.w.allan, jeffrey.t.kirsher, jesse.brandeburg
Cc: e1000-devel, netdev, prasanna.panchamukhi
Interrupts remain enabled while exiting the e100_poll(), when the driver is
down. Anything bad can happen resulting in a crash.
The solution is not to enable the interrupts while the driver is down.
Signed-off-by: Prasanna S. Panchamukhi <prasanna.panchamukhi@riverbed.com>
---
drivers/net/e100.c | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/drivers/net/e100.c b/drivers/net/e100.c
index abbf229..479219f 100644
--- a/drivers/net/e100.c
+++ b/drivers/net/e100.c
@@ -426,6 +426,7 @@ enum cb_command {
};
enum e100_state_t {
+ __E100_DOWN,
__E100_TESTING
};
@@ -2177,7 +2178,8 @@ static int e100_poll(struct napi_struct *napi, int budget)
/* If budget not fully consumed, exit the polling mode */
if (work_done < budget) {
napi_complete(napi);
- e100_enable_irq(nic);
+ if (!test_bit(__E100_DOWN, &nic->state))
+ e100_enable_irq(nic);
}
return work_done;
@@ -2230,6 +2232,7 @@ static int e100_up(struct nic *nic)
{
int err;
+ clear_bit(__E100_DOWN, &nic->state);
if ((err = e100_rx_alloc_list(nic)))
return err;
if ((err = e100_alloc_cbs(nic)))
@@ -2242,6 +2245,7 @@ static int e100_up(struct nic *nic)
if ((err = request_irq(nic->pdev->irq, e100_intr, IRQF_SHARED,
nic->netdev->name, nic->netdev)))
goto err_no_irq;
+ set_bit(__E100_DOWN, &nic->state);
netif_wake_queue(nic->netdev);
napi_enable(&nic->napi);
/* enable ints _after_ enabling poll, preventing a race between
@@ -2917,7 +2921,9 @@ err_out_free_dev:
static void __devexit e100_remove(struct pci_dev *pdev)
{
struct net_device *netdev = pci_get_drvdata(pdev);
+ struct nic *nic = netdev_priv(netdev);
+ set_bit(__E100_DOWN, &nic->state);
if (netdev) {
struct nic *nic = netdev_priv(netdev);
unregister_netdev(netdev);
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH] net: cpuvert to new cpumask API
From: KOSAKI Motohiro @ 2011-05-18 0:14 UTC (permalink / raw)
To: davem; +Cc: linux-kernel, therbert, eric.dumazet, netdev
In-Reply-To: <20110516.115357.193701317.davem@davemloft.net>
(2011/05/17 3:53), David Miller wrote:
> From: KOSAKI Motohiro<kosaki.motohiro@jp.fujitsu.com>
> Date: Thu, 28 Apr 2011 23:56:35 +0900 (JST)
>
>> We plan to remove cpu_xx() old api later. Thus this patch
>> convert it.
>>
>> This patch has no functional change.
>>
>> Signed-off-by: KOSAKI Motohiro<kosaki.motohiro@jp.fujitsu.com>
>
> Applied, thanks.
Great! :)
^ 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