* Re: the future of ethtool
From: Thomas Graf @ 2010-11-16 6:17 UTC (permalink / raw)
To: David Miller; +Cc: jeff, bhutchings, shemminger, netdev
In-Reply-To: <20101115.180225.189678628.davem@davemloft.net>
On Mon, Nov 15, 2010 at 06:02:25PM -0800, David Miller wrote:
> It isn't sufficient. You can still get into unwindable failures.
>
> Earlier operations can consume fixed resources like TCAM filter
> slots or rx/tx queues, making a subsequent operation in the
> sequence fail.
>
> A validate/commit scheme cannot detect this effectively.
I agree, there are many more scenarios where it would not work
reliably. It would have ensured that all provided values are
within range boundries and that all requested operations are in
fact supported. Since I have disregarded the idea, I does not
matter much anymore.
^ permalink raw reply
* [PATCH net-next-2.6] udp: use atomic_inc_not_zero_hint
From: Eric Dumazet @ 2010-11-16 5:58 UTC (permalink / raw)
To: David Miller; +Cc: netdev
UDP sockets refcount is usually 2, unless an incoming frame is going to
be queued in receive or backlog queue.
Using atomic_inc_not_zero_hint() permits to reduce latency, because
processor issues less memory transactions.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
include/net/sock.h | 2 +-
net/ipv4/udp.c | 4 ++--
net/ipv6/udp.c | 4 ++--
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/net/sock.h b/include/net/sock.h
index a6338d0..eb0c1f5 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -57,7 +57,7 @@
#include <linux/rculist_nulls.h>
#include <linux/poll.h>
-#include <asm/atomic.h>
+#include <linux/atomic.h>
#include <net/dst.h>
#include <net/checksum.h>
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 5e0a3a5..491ecd3 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -430,7 +430,7 @@ begin:
if (result) {
exact_match:
- if (unlikely(!atomic_inc_not_zero(&result->sk_refcnt)))
+ if (unlikely(!atomic_inc_not_zero_hint(&result->sk_refcnt, 2)))
result = NULL;
else if (unlikely(compute_score2(result, net, saddr, sport,
daddr, hnum, dif) < badness)) {
@@ -500,7 +500,7 @@ begin:
goto begin;
if (result) {
- if (unlikely(!atomic_inc_not_zero(&result->sk_refcnt)))
+ if (unlikely(!atomic_inc_not_zero_hint(&result->sk_refcnt, 2)))
result = NULL;
else if (unlikely(compute_score(result, net, saddr, hnum, sport,
daddr, dport, dif) < badness)) {
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 91def93..b541a4e 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -227,7 +227,7 @@ begin:
if (result) {
exact_match:
- if (unlikely(!atomic_inc_not_zero(&result->sk_refcnt)))
+ if (unlikely(!atomic_inc_not_zero_hint(&result->sk_refcnt, 2)))
result = NULL;
else if (unlikely(compute_score2(result, net, saddr, sport,
daddr, hnum, dif) < badness)) {
@@ -294,7 +294,7 @@ begin:
goto begin;
if (result) {
- if (unlikely(!atomic_inc_not_zero(&result->sk_refcnt)))
+ if (unlikely(!atomic_inc_not_zero_hint(&result->sk_refcnt, 2)))
result = NULL;
else if (unlikely(compute_score(result, net, hnum, saddr, sport,
daddr, dport, dif) < badness)) {
^ permalink raw reply related
* Re: [PATCH net-next-2.6 v2] can: Topcliff: PCH_CAN driver: Add Flow control,
From: Tomoya MORINAGA @ 2010-11-16 5:30 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Masayuki Ohtake,
Samuel Ortiz, margie.foster-ral2JQCrhuEAvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA, LKML,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w, Wolfgang Grandegger,
joel.clark-ral2JQCrhuEAvxtiuMwx3w, David S. Miller,
Christian Pellegrin, qi.wang-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <4CE0FF5A.3000100@pengutronix.de>
Hi Marc,
On Monday, November 15, 2010 6:37 PM, Marc Kleine-Budde wrote :
> Either use ioread8 or ioread16 in both the rx and tx path.
I will modify to ioread16
>> + if (ioread32(&priv->regs->treq2) & 0xfc00) {
> what does this if check?
This is for whether all tx requests are complete or not.
---
Thanks,
Tomoya MORINAGA
OKI SEMICONDUCTOR CO., LTD.
^ permalink raw reply
* Re: [PATCH net-next-2.6 v2] can: Topcliff: PCH_CAN driver: Add Flow control,
From: Tomoya MORINAGA @ 2010-11-16 5:18 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Masayuki Ohtake,
Samuel Ortiz, margie.foster-ral2JQCrhuEAvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA, LKML,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w, Wolfgang Grandegger,
joel.clark-ral2JQCrhuEAvxtiuMwx3w, David S. Miller,
Christian Pellegrin, qi.wang-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <4CE0F92A.40308@pengutronix.de>
On Monday, November 15, 2010 6:11 PM, Marc Kleine-Budde wrote:
> Please make it one patch per topic. A review of your patch will follow.
It's too late now.
This is difficult to separete patch per topic from current our CAN driver.
Next time posting another patch, I will do like so.
---
Thanks,
Tomoya MORINAGA
OKI SEMICONDUCTOR CO., LTD.
^ permalink raw reply
* [PATCH] net/unix: Allow Unix sockets to be treated like normal files.
From: Jeff Hansen @ 2010-11-16 4:23 UTC (permalink / raw)
To: netdev; +Cc: Dave Miller, Jeff Hansen
This is an update to my previous patch that fixes crashes on systems that
used filp->private_data for their own purposes.
For some reason, it doesn't look like this will ever get main-lined. But,
just in case someone wants to be able to write to a Unix socket with:
$ echo send_cmd > /tmp/my_socket
This is the patch that will let you do that.
This allows Unix sockets to be opened, written, read, and closed, like
normal files. This can be especially handy from, for example, a shell
script that wants to send a short message to a Unix socket, but doesn't
want to and/or cannot create the socket itself.
Signed-off-by: Jeff Hansen <x@jeffhansen.com>
---
net/unix/Kconfig | 10 +++
net/unix/af_unix.c | 177 ++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 187 insertions(+), 0 deletions(-)
diff --git a/net/unix/Kconfig b/net/unix/Kconfig
index 5a69733..68df4f1 100644
--- a/net/unix/Kconfig
+++ b/net/unix/Kconfig
@@ -19,3 +19,13 @@ config UNIX
Say Y unless you know what you are doing.
+config UNIX_FOPS
+ boolean "Allow Unix sockets to be treated like normal files"
+ depends on UNIX
+ ---help---
+ If you say Y here, Unix sockets may be opened, written, read, and
+ closed, like normal files. This is handy for sending short commands
+ to Unix sockets (i.e. from shell scripts), without having to create
+ a Unix socket.
+
+ Say Y unless you know what you are doing.
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 0ebc777..81ba74d 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -798,6 +798,178 @@ fail:
return NULL;
}
+#ifdef CONFIG_UNIX_FOPS
+struct sock *unix_fops_find_by_filp(struct file *filp,
+ struct sockaddr_un *sunaddr, unsigned int *_alen)
+{
+ int hash, nlen, alen, retval;
+ struct sock *sock = NULL;
+ struct net *net = &init_net;
+ char *p;
+ struct dentry *dentry = filp->f_dentry;
+
+ retval = -EINVAL;
+ if (!filp)
+ goto error;
+ dentry = filp->f_dentry;
+
+ if (!dentry || !dentry->d_parent)
+ goto error;
+
+ sunaddr->sun_family = AF_UNIX;
+ p = d_path(&filp->f_path, sunaddr->sun_path, sizeof(sunaddr->sun_path));
+ if (IS_ERR(p))
+ return (void *)p;
+ if (!p)
+ goto error;
+ nlen = strnlen(p, sizeof(sunaddr->sun_path) -
+ (p - sunaddr->sun_path));
+
+ memmove(sunaddr->sun_path, p, nlen);
+ alen = nlen;
+ if (nlen < sizeof(sunaddr->sun_path)) {
+ sunaddr->sun_path[nlen] = 0;
+ alen++;
+ }
+ alen += sizeof(sunaddr->sun_family);
+
+ if (_alen)
+ *_alen = alen;
+
+ hash = dentry->d_inode->i_ino & (UNIX_HASH_SIZE-1);
+ sock = unix_find_socket_byname(net, sunaddr, alen, 0, hash);
+
+error:
+ if (sock)
+ return sock;
+ else
+ return ERR_PTR(retval);
+}
+
+static int unix_open(struct inode *inode, struct file *filp)
+{
+ int err, alen;
+ struct socket *sock = NULL;
+ struct sock *usock;
+ struct unix_sock *u = NULL;
+ struct sockaddr_un sunaddr = { 0 };
+
+ usock = unix_fops_find_by_filp(filp, &sunaddr, &alen);
+ if (IS_ERR(usock))
+ return PTR_ERR(usock);
+ u = unix_sk(usock);
+ err = -EBUSY;
+ if (u->fops_socket)
+ goto error;
+
+ err = sock_create_kern(PF_UNIX, usock->sk_type, 0, &sock);
+ if (err)
+ goto error;
+
+ err = sock->ops->connect(sock, (struct sockaddr *)&sunaddr,
+ alen, 0);
+ if (err) {
+ sock_release(sock);
+ goto error;
+ }
+ u->fops_socket = sock;
+
+ /* FALLTHROUGH */
+error:
+ sock_put(usock);
+
+ return err;
+}
+
+static int unix_frelease(struct inode *inode, struct file *filp)
+{
+ int retval;
+ struct socket *sock;
+ struct sock *usock;
+ struct unix_sock *u;
+ struct sockaddr_un sunaddr;
+
+ usock = unix_fops_find_by_filp(filp, &sunaddr, NULL);
+ if (IS_ERR(usock))
+ return PTR_ERR(usock);
+ u = unix_sk(usock);
+ sock = u->fops_socket;
+ retval = -EINVAL;
+ if (!sock)
+ goto error;
+ u->fops_socket = NULL;
+ sock_release(sock);
+ retval = 0;
+
+ /* FALLTHROUGH */
+error:
+ sock_put(usock);
+ return retval;
+}
+
+static ssize_t unix_readwrite(struct file *filp, void *buf,
+ size_t _len, loff_t *ppos, int do_write)
+{
+ int len = (int)_len, err = 0;
+ struct kvec iov = {
+ .iov_base = buf,
+ .iov_len = len,
+ };
+ struct msghdr msg = {
+ /* NB: struct iovec and kvec are equal */
+ .msg_iov = (struct iovec *)&iov,
+ .msg_iovlen = 1,
+ };
+ struct socket *sock;
+ struct sock *usock;
+ struct unix_sock *u;
+ struct sockaddr_un sunaddr;
+
+ usock = unix_fops_find_by_filp(filp, &sunaddr, NULL);
+ if (IS_ERR(usock))
+ return PTR_ERR(usock);
+ u = unix_sk(usock);
+ sock = u->fops_socket;
+ err = -EINVAL;
+ if (!sock)
+ goto error;
+
+ err = -E2BIG;
+ if (_len > 0xffffffffLL)
+ goto error;
+
+ err = do_write ? sock_sendmsg(sock, &msg, len) :
+ sock_recvmsg(sock, &msg, len, 0);
+ if (err > 0 && ppos)
+ *ppos += err;
+
+ /* FALLTHROUGH */
+error:
+ sock_put(usock);
+
+ return err;
+}
+
+static ssize_t unix_write(struct file *filp, const char __user *buf,
+ size_t _len, loff_t *ppos)
+{
+ return unix_readwrite(filp, (void *)buf, _len, ppos, 1);
+}
+
+static ssize_t unix_read(struct file *filp, char __user *buf,
+ size_t _len, loff_t *ppos)
+{
+ return unix_readwrite(filp, (void *)buf, _len, ppos, 0);
+}
+
+const struct file_operations unix_sock_fops = {
+ .owner = THIS_MODULE,
+ .open = unix_open,
+ .release = unix_frelease,
+ .write = unix_write,
+ .read = unix_read,
+};
+#endif /* CONFIG_UNIX_FOPS */
static int unix_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
{
@@ -874,6 +1046,11 @@ out_mknod_drop_write:
mnt_drop_write(nd.path.mnt);
if (err)
goto out_mknod_dput;
+
+#ifdef CONFIG_UNIX_FOPS
+ dentry->d_inode->i_fop = &unix_sock_fops;
+#endif
+
mutex_unlock(&nd.path.dentry->d_inode->i_mutex);
dput(nd.path.dentry);
nd.path.dentry = dentry;
--
1.7.0.4
^ permalink raw reply related
* Re: linux-next: build failure after merge of the net tree
From: David Miller @ 2010-11-16 4:15 UTC (permalink / raw)
To: sfr; +Cc: netdev, linux-next, linux-kernel, jesse
In-Reply-To: <20101116113437.4f536d16.sfr@canb.auug.org.au>
From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Tue, 16 Nov 2010 11:34:37 +1100
> After merging the net tree, today's linux-next build (x86_64 allmodconfig)
> failed like this:
>
> ERROR: "netif_get_vlan_features" [drivers/net/xen-netfront.ko] undefined!
>
> Caused by commit 58e998c6d23988490162cef0784b19ea274d90bb ("offloading:
> Force software GSO for multiple vlan tags").
>
> Presumably netif_get_vlan_features needs exporting.
Thanks Stephen, I've just pushed the following fix.
--------------------
net: Export netif_get_vlan_features().
ERROR: "netif_get_vlan_features" [drivers/net/xen-netfront.ko] undefined!
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
net/core/dev.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 8725d16..381b8e2 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1983,6 +1983,7 @@ int netif_get_vlan_features(struct sk_buff *skb, struct net_device *dev)
else
return 0;
}
+EXPORT_SYMBOL(netif_get_vlan_features);
/*
* Returns true if either:
--
1.7.3.2
^ permalink raw reply related
* Re: Remaining problems in firewire-net
From: Maxim Levitsky @ 2010-11-16 3:31 UTC (permalink / raw)
To: Stefan Richter; +Cc: netdev, linux1394-devel, linux-kernel
In-Reply-To: <20101115090103.0df0b5ef@stein>
On Mon, 2010-11-15 at 09:01 +0100, Stefan Richter wrote:
> On Nov 15 Maxim Levitsky wrote:
> > That is because 1394 spec specifies that first of all the ISO channel
> > must be allocated from the IRM node. The firewire stack currently just
> > uses hardcoded numbers in two places the ISO is used
> > (firewire-net, and firedtv)
> > However it has all functions implemented for this.
>
> This is a bug (missing feature) in firedtv but not in firewire-net. The
> broadcast channel is allocated and reallocated by the bus manager, not
> by an IP-over-1394 user. But you found that out already, below.
Agree fully.
>
> Channel allocation and DMA context allocation and control are unrelated
> issues, on the other hand. One is a higher-level cooperative protocol
> for bus-wide resource management (in which the nodes' roles are defined
> in various different ways by protocols such as AV/C's CMP or by IIDC).
> The other is about hardware control locally in the OHCI bus bridge
> hardware.
>
> [...]
> > In case of firewire-net, it is simpler, because it uses the broadcast
> > channel, so it only has to find who is the IRM and read its
> > BROADCAST_CHANNEL.
> >
> > However, I think I need to write a function to query the IRM its
> > broadcast channel, don't think it has one.
>
> Overkill. Just leave it as is:
> 1.) We know which number the broadcast channel has.
> 2.) We optimistically assume that a 1394a compliant IRM or bus
> manager exists and allocated that channel.
>
> Why introduce entirely unnecessary fragility?
Looking at spec again, indeed 31 the the default broadcast channel, and
probably nobody ever changes it by writing the BROADCAST_CHANNEL.
The BROADCAST_CHANNEL register was actually added in 1394a.
>
> > Speaking of IRM discovery, the spec says it should be a node with
> > contender bit and largest node id. However, the code in
> > core-topology.c, build_tree seems to take the node that sent the
> > selfID packet last.
>
> This is because of a monotony rule of how self ID packets arrive during
> self identification phase.
Ah, ok, found this bit in spec too.
Btw, according to spec the firewire appears to be half-duplex.
Also one note I wanted to say in previous mail. but forgot.
The IP over 1394 tells that unicast can also use ISO transactions.
However, it doesn't say how to negotiate the ISO channel number, thus a
logical conclusion is that its not possible to use ISO transactions for
unicast.
Is that right?
Best regards,
Maxim Levitsky
------------------------------------------------------------------------------
Beautiful is writing same markup. Internet Explorer 9 supports
standards for HTML5, CSS3, SVG 1.1, ECMAScript5, and DOM L2 & L3.
Spend less time writing and rewriting code and more time creating great
experiences on the web. Be a part of the beta today
http://p.sf.net/sfu/msIE9-sfdev2dev
^ permalink raw reply
* Re: patchwork deferred?
From: Joe Perches @ 2010-11-16 2:16 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20101115.180419.112575212.davem@davemloft.net>
On Mon, 2010-11-15 at 18:04 -0800, David Miller wrote:
> I'd like you to slow down so I can spend time on the
> more neuron intensive patches.
No worries. I'll wait awhile before submitting any
new net drivers/net trivia patches.
^ permalink raw reply
* Re: [net-2.6 PATCH] nete zero kobject in rx_queue_release
From: John Fastabend @ 2010-11-16 2:06 UTC (permalink / raw)
To: David Miller
Cc: therbert@google.com, netdev@vger.kernel.org,
eric.dumazet@gmail.com
In-Reply-To: <20101114.151529.183053743.davem@davemloft.net>
On 11/14/2010 3:15 PM, David Miller wrote:
> From: Tom Herbert <therbert@google.com>
> Date: Sun, 14 Nov 2010 14:40:00 -0800
>
>>> So we'll need something like:
>>>
>>> if (atomic_dec_and_test(&first->count))
>>> kfree(first);
>>> else
>>> /* clear everything except queue->first */
>>>
>>
>> The patches to get rid of the separate refcnt should obviate this
>> complexity. Could just clear the queue in kobject release.
>
> True but we'll still need a patch for older kernels.
OK Thanks. I'll have a stable patch and a net-2.6 patch soon.
-- John
^ permalink raw reply
* Re: network device reference leak with net-next
From: David Miller @ 2010-11-16 2:06 UTC (permalink / raw)
To: eric.dumazet
Cc: john.r.fastabend, shemminger, lorenzo, netdev, brian.haley, maze
In-Reply-To: <1289863543.3364.24.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 16 Nov 2010 00:25:43 +0100
> Le lundi 15 novembre 2010 à 15:18 -0800, John Fastabend a écrit :
>> With a quick hack the above seems to resolve the issue but I'll need to review the ipv6 stuff to be sure this is sane.
>>
>> Thanks,
>> John.
>
> Your machine is faster than mine ;)
>
> Yes this commit seems buggy, thanks for finding the problem !
Indeed, thanks John. Please send a final fix after you've done
your review.
Thanks again!
^ permalink raw reply
* Re: patchwork deferred?
From: David Miller @ 2010-11-16 2:04 UTC (permalink / raw)
To: joe; +Cc: netdev
In-Reply-To: <1289869813.16461.173.camel@Joe-Laptop>
From: Joe Perches <joe@perches.com>
Date: Mon, 15 Nov 2010 17:10:13 -0800
> Now that net-next is open, should patches with patchwork
> status "deferred" be resubmitted?
>From you, no, you're already consuming the bulk of my queue
and I'd like you to slow down so I can spend time on the
more neuron intensive patches.
Thanks.
^ permalink raw reply
* Re: the future of ethtool
From: David Miller @ 2010-11-16 2:02 UTC (permalink / raw)
To: tgraf; +Cc: jeff, bhutchings, shemminger, netdev
In-Reply-To: <20101115233335.GB24292@canuck.infradead.org>
From: Thomas Graf <tgraf@infradead.org>
Date: Mon, 15 Nov 2010 18:33:35 -0500
> I tried to solve this by splitting the validate/change operation and
> thus be able to validate all requests before starting to commit
> them. This would mean changing all drivers though which I wasn't
> willing to do.
It isn't sufficient. You can still get into unwindable failures.
Earlier operations can consume fixed resources like TCAM filter
slots or rx/tx queues, making a subsequent operation in the
sequence fail.
A validate/commit scheme cannot detect this effectively.
^ permalink raw reply
* Re: [PATCH] net: use the macros defined for the members of flowi
From: David Miller @ 2010-11-16 1:56 UTC (permalink / raw)
To: eric.dumazet; +Cc: brian.haley, xiaosuo, netdev
In-Reply-To: <1289857126.3364.14.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 15 Nov 2010 22:38:46 +0100
> Le lundi 15 novembre 2010 à 16:33 -0500, Brian Haley a écrit :
>> On 11/12/2010 11:43 PM, Changli Gao wrote:
>> > Use the macros defined for the members of flowi to clean the code up.
>> >
>> > diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
>> > index 865fd76..36cd0b7 100644
>> > --- a/net/bridge/br_netfilter.c
>> > +++ b/net/bridge/br_netfilter.c
>> > @@ -412,13 +412,8 @@ static int br_nf_pre_routing_finish(struct sk_buff *skb)
>> > if (dnat_took_place(skb)) {
>> > if ((err = ip_route_input(skb, iph->daddr, iph->saddr, iph->tos, dev))) {
>> > struct flowi fl = {
>> > - .nl_u = {
>> > - .ip4_u = {
>> > - .daddr = iph->daddr,
>> > - .saddr = 0,
>> > - .tos = RT_TOS(iph->tos) },
>> > - },
>> > - .proto = 0,
>> > + .fl4_dst = iph->daddr,
>> > + .fl4_tos = RT_TOS(iph->tos),
>> > };
>>
>> Are these actually equivalent? You dropped two assignments to zero.
>> I always thought things on the stack weren't.
>
> Same question on lkml few hours ago. I think gcc does the assignement to
> zero, even on automatic variables (at least done on x86), but could not
> find a doc on it.
It always has, it always will. "type var = { };" always zeros
out the structure.
^ permalink raw reply
* patchwork deferred?
From: Joe Perches @ 2010-11-16 1:10 UTC (permalink / raw)
To: David Miller; +Cc: netdev
Now that net-next is open, should patches with patchwork
status "deferred" be resubmitted?
^ permalink raw reply
* linux-next: build failure after merge of the net tree
From: Stephen Rothwell @ 2010-11-16 0:34 UTC (permalink / raw)
To: David Miller, netdev; +Cc: linux-next, linux-kernel, Jesse Gross
[-- Attachment #1: Type: text/plain, Size: 582 bytes --]
Hi all,
After merging the net tree, today's linux-next build (x86_64 allmodconfig)
failed like this:
ERROR: "netif_get_vlan_features" [drivers/net/xen-netfront.ko] undefined!
Caused by commit 58e998c6d23988490162cef0784b19ea274d90bb ("offloading:
Force software GSO for multiple vlan tags").
Presumably netif_get_vlan_features needs exporting.
I have used the net tree from next-20101115 for today (with
1d7138de878d1d4210727c1200193e69596f93b3 reverted).
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: the future of ethtool
From: Ben Hutchings @ 2010-11-16 0:10 UTC (permalink / raw)
To: Thomas Graf; +Cc: Jeff Garzik, Stephen Hemminger, NetDev, David Miller
In-Reply-To: <20101115233335.GB24292@canuck.infradead.org>
On Mon, 2010-11-15 at 18:33 -0500, Thomas Graf wrote:
> On Mon, Nov 15, 2010 at 05:49:33PM -0500, Jeff Garzik wrote:
> > s/only// I don't think Stephen is suggesting sending _some_ ops
> > through netlink and others through old-ioctl. That's just silly.
> > Any new netlink interface should transit all existing ETHTOOL_xxx
> > commands/structures.
> >
> > But presumably, one would have the ability to send multiple
> > ETHTOOL_xxx bundled together into a single netlink transaction,
> > facilitating the kernel's calling of struct ethtool_ops'
> > ->begin()
> > ... first operation specified by userspace via netlink ...
> > ... second operation specified by userspace via netlink ...
> > ... etc.
> > ->end()
> >
> > The underlying struct ethtool_ops remains unchanged; you're only
> > changing the transit method.
> >
> > Thus, the ethtool userspace utility would switch entirely to
> > netlink, while the ioctl processing code remains for binary
> > compatibility.
> >
> > Or... ethtool userspace utility could remain unchanged, and a new
> > 'nictool' utility provides the same features but with (a) a new CLI
> > and (b) exclusively uses netlink rather than ioctl.
>
> I actually have code for this including userspace. I never submitted
> it because I wasn't confident it is the way to go since it literally
> duplicates all ethtool code in the kernel.
>
> There is one major problem with bundling multiple requests though. If
> one change request fails but other changes have been committed already
> we can't really undo them without causing lots of races. We have to
> leave the device in a somewhat inconsistent state. It's even difficult
> to tell what has been comitted and what hasn't. It also makes error
> reporting more difficult as a -ERANGE error code could apply to any
> of the values to be changed.
[...]
I think it's hopeless to make this truly transactional. Unless the
ethtool core maintains all the settings in one giant structure and
passes them over to the driver to check and apply then there is no way
driver authors are going to get it right in general. And if the ethtool
core does that then, as you say, error reporting is going to be
terrible. There will be even more need to go look in the kernel log to
see the driver's explanation of why the settings are invalid which was
too long to fit in this margin^Wreturn code.
I would expect to treat each operation in a multiple-set as conditional
on the success of all previous operations. ethtool or other utilities
should then take care to put operations in a sensible order (e.g. enable
TX checksum before TSO, if those remain separate operations). Error
reporting in the core is then as simple as reporting how many operations
were successful plus the error code for the one that failed.
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: the future of ethtool
From: Jeff Garzik @ 2010-11-16 0:07 UTC (permalink / raw)
To: Ben Hutchings, Stephen Hemminger, NetDev, David Miller
In-Reply-To: <20101115233335.GB24292@canuck.infradead.org>
On 11/15/2010 06:33 PM, Thomas Graf wrote:
> On Mon, Nov 15, 2010 at 05:49:33PM -0500, Jeff Garzik wrote:
>> s/only// I don't think Stephen is suggesting sending _some_ ops
>> through netlink and others through old-ioctl. That's just silly.
>> Any new netlink interface should transit all existing ETHTOOL_xxx
>> commands/structures.
>>
>> But presumably, one would have the ability to send multiple
>> ETHTOOL_xxx bundled together into a single netlink transaction,
>> facilitating the kernel's calling of struct ethtool_ops'
>> ->begin()
>> ... first operation specified by userspace via netlink ...
>> ... second operation specified by userspace via netlink ...
>> ... etc.
>> ->end()
>>
>> The underlying struct ethtool_ops remains unchanged; you're only
>> changing the transit method.
>>
>> Thus, the ethtool userspace utility would switch entirely to
>> netlink, while the ioctl processing code remains for binary
>> compatibility.
>>
>> Or... ethtool userspace utility could remain unchanged, and a new
>> 'nictool' utility provides the same features but with (a) a new CLI
>> and (b) exclusively uses netlink rather than ioctl.
>
> I actually have code for this including userspace. I never submitted
> it because I wasn't confident it is the way to go since it literally
> duplicates all ethtool code in the kernel.
>
> There is one major problem with bundling multiple requests though. If
> one change request fails but other changes have been committed already
> we can't really undo them without causing lots of races. We have to
> leave the device in a somewhat inconsistent state. It's even difficult
> to tell what has been comitted and what hasn't. It also makes error
> reporting more difficult as a -ERANGE error code could apply to any
> of the values to be changed.
Well, what are the range of possibilities for the _hardware_, given the
current struct ethtool_ops software interface?
We can either reset+restart RXTX after such events, or not.
That's a binary decision, one easily be passed in from userspace before
any ethtool ops are executed.
Further down the road, if one wanted to travel that far, we could save
the hardware state at the beginning, and restore hardware state if
anything fails. Depends on peoples' motivation over this rare issue.
We already save/restore hardware state for suspend/resume, so this does
not seem overly onerous.
Jeff
^ permalink raw reply
* Re: the future of ethtool
From: Thomas Graf @ 2010-11-15 23:33 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Ben Hutchings, Stephen Hemminger, NetDev, David Miller
In-Reply-To: <4CE1B8FD.3000007@garzik.org>
On Mon, Nov 15, 2010 at 05:49:33PM -0500, Jeff Garzik wrote:
> s/only// I don't think Stephen is suggesting sending _some_ ops
> through netlink and others through old-ioctl. That's just silly.
> Any new netlink interface should transit all existing ETHTOOL_xxx
> commands/structures.
>
> But presumably, one would have the ability to send multiple
> ETHTOOL_xxx bundled together into a single netlink transaction,
> facilitating the kernel's calling of struct ethtool_ops'
> ->begin()
> ... first operation specified by userspace via netlink ...
> ... second operation specified by userspace via netlink ...
> ... etc.
> ->end()
>
> The underlying struct ethtool_ops remains unchanged; you're only
> changing the transit method.
>
> Thus, the ethtool userspace utility would switch entirely to
> netlink, while the ioctl processing code remains for binary
> compatibility.
>
> Or... ethtool userspace utility could remain unchanged, and a new
> 'nictool' utility provides the same features but with (a) a new CLI
> and (b) exclusively uses netlink rather than ioctl.
I actually have code for this including userspace. I never submitted
it because I wasn't confident it is the way to go since it literally
duplicates all ethtool code in the kernel.
There is one major problem with bundling multiple requests though. If
one change request fails but other changes have been committed already
we can't really undo them without causing lots of races. We have to
leave the device in a somewhat inconsistent state. It's even difficult
to tell what has been comitted and what hasn't. It also makes error
reporting more difficult as a -ERANGE error code could apply to any
of the values to be changed.
I tried to solve this by splitting the validate/change operation and
thus be able to validate all requests before starting to commit
them. This would mean changing all drivers though which I wasn't
willing to do.
I can clean up what I have and submit it so we have something to
start with.
^ permalink raw reply
* Re: network device reference leak with net-next
From: Eric Dumazet @ 2010-11-15 23:25 UTC (permalink / raw)
To: John Fastabend
Cc: Stephen Hemminger, lorenzo, netdev@vger.kernel.org, brian.haley,
maze
In-Reply-To: <4CE1BFCE.70105@intel.com>
Le lundi 15 novembre 2010 à 15:18 -0800, John Fastabend a écrit :
> On 11/15/2010 11:10 AM, Stephen Hemminger wrote:
> > On Mon, 15 Nov 2010 20:04:00 +0100
> > Eric Dumazet <eric.dumazet@gmail.com> wrote:
> >
> >> Le lundi 15 novembre 2010 à 10:56 -0800, Stephen Hemminger a écrit :
> >>> This is a new regression (doesn't exist with 2.6.36)
> >>>
> >>> If I shutdown KVM instance with Virt manager, the virtual
> >>> interfaces in the bridge aren't getting cleaned up because
> >>> of leftover reference count.
> >>>
> >>>
> >>> [ 9781.050474] unregister_netdevice: waiting for vnet1 to become free. Usage count = 1
> >>> [ 9785.143400] virbr4: port 3(vnet6) entering forwarding state
> >>> [ 9785.177194] virbr4: port 3(vnet6) entering disabled state
> >>> [ 9785.201129] device vnet6 left promiscuous mode
> >>> [ 9785.201135] virbr4: port 3(vnet6) entering disabled state
> >>> [ 9791.286950] unregister_netdevice: waiting for vnet1 to become free. Usage count = 1
> >>> [ 9795.461526] unregister_netdevice: waiting for vnet6 to become free. Usage count = 1
> >>> [ 9801.523398] unregister_netdevice: waiting for vnet1 to become free. Usage count = 1
> >>> --
> >>
> >> Is the refcount stay forever to 1, or eventually reaches 0 ?
> >>
> >>
> >>
> >
> > Stays 1 for as long as I waited about 10 minutes
> >
>
> Similar refcount error with ixgbe bisected to this patch,
>
> commit 2de795707294972f6c34bae9de713e502c431296
> Author: Lorenzo Colitti <lorenzo@google.com>
> Date: Wed Oct 27 18:16:49 2010 +0000
>
> ipv6: addrconf: don't remove address state on ifdown if the address is being kept
>
> Currently, addrconf_ifdown does not delete statically configured IPv6
> addresses when the interface is brought down. The intent is that when
> the interface comes back up the address will be usable again. However,
> this doesn't actually work, because the system stops listening on the
> corresponding solicited-node multicast address, so the address cannot
> respond to neighbor solicitations and thus receive traffic. Also, the
> code notifies the rest of the system that the address is being deleted
> (e.g, RTM_DELADDR), even though it is not. Fix it so that none of this
> state is updated if the address is being kept on the interface.
>
> Tested: Added a statically configured IPv6 address to an interface,
> started ping, brought link down, brought link up again. When link came
> up ping kept on going and "ip -6 maddr" showed that the host was still
> subscribed to there
>
> Signed-off-by: Lorenzo Colitti <lorenzo@google.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
>
>
> Quick glance looks like an in6_ifa_put is missed?
>
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -2754,13 +2754,13 @@ Hunk #1, a/net/ipv6/addrconf.c static int addrconf_ifdown(struct net_device *dev, int how)
> ifa->state = INET6_IFADDR_STATE_DEAD;
> spin_unlock_bh(&ifa->state_lock);
>
> - if (state == INET6_IFADDR_STATE_DEAD) {
> - in6_ifa_put(ifa);
> - } else {
> + if (state != INET6_IFADDR_STATE_DEAD) {
> __ipv6_ifa_notify(RTM_DELADDR, ifa);
> atomic_notifier_call_chain(&inet6addr_chain,
> NETDEV_DOWN, ifa);
> }
> +
> + in6_ifa_put(ifa);
> write_lock_bh(&idev->lock);
> }
> }
>
> With a quick hack the above seems to resolve the issue but I'll need to review the ipv6 stuff to be sure this is sane.
>
> Thanks,
> John.
Your machine is faster than mine ;)
Yes this commit seems buggy, thanks for finding the problem !
^ permalink raw reply
* Re: network device reference leak with net-next
From: John Fastabend @ 2010-11-15 23:18 UTC (permalink / raw)
To: Stephen Hemminger, lorenzo
Cc: Eric Dumazet, netdev@vger.kernel.org, brian.haley, maze
In-Reply-To: <20101115111024.3b3377be@nehalam>
On 11/15/2010 11:10 AM, Stephen Hemminger wrote:
> On Mon, 15 Nov 2010 20:04:00 +0100
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>> Le lundi 15 novembre 2010 à 10:56 -0800, Stephen Hemminger a écrit :
>>> This is a new regression (doesn't exist with 2.6.36)
>>>
>>> If I shutdown KVM instance with Virt manager, the virtual
>>> interfaces in the bridge aren't getting cleaned up because
>>> of leftover reference count.
>>>
>>>
>>> [ 9781.050474] unregister_netdevice: waiting for vnet1 to become free. Usage count = 1
>>> [ 9785.143400] virbr4: port 3(vnet6) entering forwarding state
>>> [ 9785.177194] virbr4: port 3(vnet6) entering disabled state
>>> [ 9785.201129] device vnet6 left promiscuous mode
>>> [ 9785.201135] virbr4: port 3(vnet6) entering disabled state
>>> [ 9791.286950] unregister_netdevice: waiting for vnet1 to become free. Usage count = 1
>>> [ 9795.461526] unregister_netdevice: waiting for vnet6 to become free. Usage count = 1
>>> [ 9801.523398] unregister_netdevice: waiting for vnet1 to become free. Usage count = 1
>>> --
>>
>> Is the refcount stay forever to 1, or eventually reaches 0 ?
>>
>>
>>
>
> Stays 1 for as long as I waited about 10 minutes
>
Similar refcount error with ixgbe bisected to this patch,
commit 2de795707294972f6c34bae9de713e502c431296
Author: Lorenzo Colitti <lorenzo@google.com>
Date: Wed Oct 27 18:16:49 2010 +0000
ipv6: addrconf: don't remove address state on ifdown if the address is being kept
Currently, addrconf_ifdown does not delete statically configured IPv6
addresses when the interface is brought down. The intent is that when
the interface comes back up the address will be usable again. However,
this doesn't actually work, because the system stops listening on the
corresponding solicited-node multicast address, so the address cannot
respond to neighbor solicitations and thus receive traffic. Also, the
code notifies the rest of the system that the address is being deleted
(e.g, RTM_DELADDR), even though it is not. Fix it so that none of this
state is updated if the address is being kept on the interface.
Tested: Added a statically configured IPv6 address to an interface,
started ping, brought link down, brought link up again. When link came
up ping kept on going and "ip -6 maddr" showed that the host was still
subscribed to there
Signed-off-by: Lorenzo Colitti <lorenzo@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Quick glance looks like an in6_ifa_put is missed?
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2754,13 +2754,13 @@ Hunk #1, a/net/ipv6/addrconf.c static int addrconf_ifdown(struct net_device *dev, int how)
ifa->state = INET6_IFADDR_STATE_DEAD;
spin_unlock_bh(&ifa->state_lock);
- if (state == INET6_IFADDR_STATE_DEAD) {
- in6_ifa_put(ifa);
- } else {
+ if (state != INET6_IFADDR_STATE_DEAD) {
__ipv6_ifa_notify(RTM_DELADDR, ifa);
atomic_notifier_call_chain(&inet6addr_chain,
NETDEV_DOWN, ifa);
}
+
+ in6_ifa_put(ifa);
write_lock_bh(&idev->lock);
}
}
With a quick hack the above seems to resolve the issue but I'll need to review the ipv6 stuff to be sure this is sane.
Thanks,
John.
^ permalink raw reply
* Re: [PATCH 07/10] drivers/net/vxge: Remove unnecessary casts of netdev_priv
From: Jon Mason @ 2010-11-15 22:51 UTC (permalink / raw)
To: Joe Perches
Cc: Jiri Kosina, Ramkrishna Vepa, Sivakumar Subramani,
Sreenivasa Honnur, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <5aee4bf89b5baa6e1d32c99be560893925ed2e19.1289855436.git.joe@perches.com>
On Mon, Nov 15, 2010 at 01:12:30PM -0800, Joe Perches wrote:
> Signed-off-by: Joe Perches <joe@perches.com>
Looks good to me.
Acked-by: Jon Mason <jon.mason@exar.com>
> ---
> drivers/net/vxge/vxge-ethtool.c | 26 +++++++++++++-------------
> drivers/net/vxge/vxge-main.c | 26 +++++++++++++-------------
> 2 files changed, 26 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/net/vxge/vxge-ethtool.c b/drivers/net/vxge/vxge-ethtool.c
> index 09f721e..bc9bd10 100644
> --- a/drivers/net/vxge/vxge-ethtool.c
> +++ b/drivers/net/vxge/vxge-ethtool.c
> @@ -80,7 +80,7 @@ static int vxge_ethtool_gset(struct net_device *dev, struct ethtool_cmd *info)
> static void vxge_ethtool_gdrvinfo(struct net_device *dev,
> struct ethtool_drvinfo *info)
> {
> - struct vxgedev *vdev = (struct vxgedev *)netdev_priv(dev);
> + struct vxgedev *vdev = netdev_priv(dev);
> strlcpy(info->driver, VXGE_DRIVER_NAME, sizeof(VXGE_DRIVER_NAME));
> strlcpy(info->version, DRV_VERSION, sizeof(DRV_VERSION));
> strlcpy(info->fw_version, vdev->fw_version, VXGE_HW_FW_STRLEN);
> @@ -108,7 +108,7 @@ static void vxge_ethtool_gregs(struct net_device *dev,
> enum vxge_hw_status status;
> u64 reg;
> u64 *reg_space = (u64 *)space;
> - struct vxgedev *vdev = (struct vxgedev *)netdev_priv(dev);
> + struct vxgedev *vdev = netdev_priv(dev);
> struct __vxge_hw_device *hldev = vdev->devh;
>
> regs->len = sizeof(struct vxge_hw_vpath_reg) * vdev->no_of_vpath;
> @@ -144,7 +144,7 @@ static void vxge_ethtool_gregs(struct net_device *dev,
> */
> static int vxge_ethtool_idnic(struct net_device *dev, u32 data)
> {
> - struct vxgedev *vdev = (struct vxgedev *)netdev_priv(dev);
> + struct vxgedev *vdev = netdev_priv(dev);
> struct __vxge_hw_device *hldev = vdev->devh;
>
> vxge_hw_device_flick_link_led(hldev, VXGE_FLICKER_ON);
> @@ -166,7 +166,7 @@ static int vxge_ethtool_idnic(struct net_device *dev, u32 data)
> static void vxge_ethtool_getpause_data(struct net_device *dev,
> struct ethtool_pauseparam *ep)
> {
> - struct vxgedev *vdev = (struct vxgedev *)netdev_priv(dev);
> + struct vxgedev *vdev = netdev_priv(dev);
> struct __vxge_hw_device *hldev = vdev->devh;
>
> vxge_hw_device_getpause_data(hldev, 0, &ep->tx_pause, &ep->rx_pause);
> @@ -185,7 +185,7 @@ static void vxge_ethtool_getpause_data(struct net_device *dev,
> static int vxge_ethtool_setpause_data(struct net_device *dev,
> struct ethtool_pauseparam *ep)
> {
> - struct vxgedev *vdev = (struct vxgedev *)netdev_priv(dev);
> + struct vxgedev *vdev = netdev_priv(dev);
> struct __vxge_hw_device *hldev = vdev->devh;
>
> vxge_hw_device_setpause_data(hldev, 0, ep->tx_pause, ep->rx_pause);
> @@ -203,7 +203,7 @@ static void vxge_get_ethtool_stats(struct net_device *dev,
> enum vxge_hw_status status;
> enum vxge_hw_status swstatus;
> struct vxge_vpath *vpath = NULL;
> - struct vxgedev *vdev = (struct vxgedev *)netdev_priv(dev);
> + struct vxgedev *vdev = netdev_priv(dev);
> struct __vxge_hw_device *hldev = vdev->devh;
> struct vxge_hw_xmac_stats *xmac_stats;
> struct vxge_hw_device_stats_sw_info *sw_stats;
> @@ -572,7 +572,7 @@ static void vxge_ethtool_get_strings(struct net_device *dev, u32 stringset,
> {
> int stat_size = 0;
> int i, j;
> - struct vxgedev *vdev = (struct vxgedev *)netdev_priv(dev);
> + struct vxgedev *vdev = netdev_priv(dev);
> switch (stringset) {
> case ETH_SS_STATS:
> vxge_add_string("VPATH STATISTICS%s\t\t\t",
> @@ -1059,21 +1059,21 @@ static void vxge_ethtool_get_strings(struct net_device *dev, u32 stringset,
>
> static int vxge_ethtool_get_regs_len(struct net_device *dev)
> {
> - struct vxgedev *vdev = (struct vxgedev *)netdev_priv(dev);
> + struct vxgedev *vdev = netdev_priv(dev);
>
> return sizeof(struct vxge_hw_vpath_reg) * vdev->no_of_vpath;
> }
>
> static u32 vxge_get_rx_csum(struct net_device *dev)
> {
> - struct vxgedev *vdev = (struct vxgedev *)netdev_priv(dev);
> + struct vxgedev *vdev = netdev_priv(dev);
>
> return vdev->rx_csum;
> }
>
> static int vxge_set_rx_csum(struct net_device *dev, u32 data)
> {
> - struct vxgedev *vdev = (struct vxgedev *)netdev_priv(dev);
> + struct vxgedev *vdev = netdev_priv(dev);
>
> if (data)
> vdev->rx_csum = 1;
> @@ -1095,7 +1095,7 @@ static int vxge_ethtool_op_set_tso(struct net_device *dev, u32 data)
>
> static int vxge_ethtool_get_sset_count(struct net_device *dev, int sset)
> {
> - struct vxgedev *vdev = (struct vxgedev *)netdev_priv(dev);
> + struct vxgedev *vdev = netdev_priv(dev);
>
> switch (sset) {
> case ETH_SS_STATS:
> @@ -1114,7 +1114,7 @@ static int vxge_ethtool_get_sset_count(struct net_device *dev, int sset)
>
> static int vxge_set_flags(struct net_device *dev, u32 data)
> {
> - struct vxgedev *vdev = (struct vxgedev *)netdev_priv(dev);
> + struct vxgedev *vdev = netdev_priv(dev);
> enum vxge_hw_status status;
>
> if (data & ~ETH_FLAG_RXHASH)
> @@ -1148,7 +1148,7 @@ static int vxge_set_flags(struct net_device *dev, u32 data)
>
> static int vxge_fw_flash(struct net_device *dev, struct ethtool_flash *parms)
> {
> - struct vxgedev *vdev = (struct vxgedev *)netdev_priv(dev);
> + struct vxgedev *vdev = netdev_priv(dev);
>
> if (vdev->max_vpath_supported != VXGE_HW_MAX_VIRTUAL_PATHS) {
> printk(KERN_INFO "Single Function Mode is required to flash the"
> diff --git a/drivers/net/vxge/vxge-main.c b/drivers/net/vxge/vxge-main.c
> index 22c7d79..5cba4a6 100644
> --- a/drivers/net/vxge/vxge-main.c
> +++ b/drivers/net/vxge/vxge-main.c
> @@ -153,7 +153,7 @@ static void
> vxge_callback_link_up(struct __vxge_hw_device *hldev)
> {
> struct net_device *dev = hldev->ndev;
> - struct vxgedev *vdev = (struct vxgedev *)netdev_priv(dev);
> + struct vxgedev *vdev = netdev_priv(dev);
>
> vxge_debug_entryexit(VXGE_TRACE, "%s: %s:%d",
> vdev->ndev->name, __func__, __LINE__);
> @@ -177,7 +177,7 @@ static void
> vxge_callback_link_down(struct __vxge_hw_device *hldev)
> {
> struct net_device *dev = hldev->ndev;
> - struct vxgedev *vdev = (struct vxgedev *)netdev_priv(dev);
> + struct vxgedev *vdev = netdev_priv(dev);
>
> vxge_debug_entryexit(VXGE_TRACE,
> "%s: %s:%d", vdev->ndev->name, __func__, __LINE__);
> @@ -787,7 +787,7 @@ vxge_xmit(struct sk_buff *skb, struct net_device *dev)
> return NETDEV_TX_OK;
> }
>
> - vdev = (struct vxgedev *)netdev_priv(dev);
> + vdev = netdev_priv(dev);
>
> if (unlikely(!is_vxge_card_up(vdev))) {
> vxge_debug_tx(VXGE_ERR,
> @@ -1052,7 +1052,7 @@ static void vxge_set_multicast(struct net_device *dev)
> vxge_debug_entryexit(VXGE_TRACE,
> "%s:%d", __func__, __LINE__);
>
> - vdev = (struct vxgedev *)netdev_priv(dev);
> + vdev = netdev_priv(dev);
> hldev = (struct __vxge_hw_device *)vdev->devh;
>
> if (unlikely(!is_vxge_card_up(vdev)))
> @@ -1209,7 +1209,7 @@ static int vxge_set_mac_addr(struct net_device *dev, void *p)
>
> vxge_debug_entryexit(VXGE_TRACE, "%s:%d", __func__, __LINE__);
>
> - vdev = (struct vxgedev *)netdev_priv(dev);
> + vdev = netdev_priv(dev);
> hldev = vdev->devh;
>
> if (!is_valid_ether_addr(addr->sa_data))
> @@ -1671,7 +1671,7 @@ static void vxge_netpoll(struct net_device *dev)
> struct __vxge_hw_device *hldev;
> struct vxgedev *vdev;
>
> - vdev = (struct vxgedev *)netdev_priv(dev);
> + vdev = netdev_priv(dev);
> hldev = pci_get_drvdata(vdev->pdev);
>
> vxge_debug_entryexit(VXGE_TRACE, "%s:%d", __func__, __LINE__);
> @@ -2581,7 +2581,7 @@ vxge_open(struct net_device *dev)
> vxge_debug_entryexit(VXGE_TRACE,
> "%s: %s:%d", dev->name, __func__, __LINE__);
>
> - vdev = (struct vxgedev *)netdev_priv(dev);
> + vdev = netdev_priv(dev);
> hldev = pci_get_drvdata(vdev->pdev);
> function_mode = vdev->config.device_hw_info.function_mode;
>
> @@ -2809,7 +2809,7 @@ static int do_vxge_close(struct net_device *dev, int do_io)
> vxge_debug_entryexit(VXGE_TRACE, "%s: %s:%d",
> dev->name, __func__, __LINE__);
>
> - vdev = (struct vxgedev *)netdev_priv(dev);
> + vdev = netdev_priv(dev);
> hldev = pci_get_drvdata(vdev->pdev);
>
> if (unlikely(!is_vxge_card_up(vdev)))
> @@ -3138,7 +3138,7 @@ vxge_tx_watchdog(struct net_device *dev)
>
> vxge_debug_entryexit(VXGE_TRACE, "%s:%d", __func__, __LINE__);
>
> - vdev = (struct vxgedev *)netdev_priv(dev);
> + vdev = netdev_priv(dev);
>
> vdev->cric_err_event = VXGE_HW_EVENT_RESET_START;
>
> @@ -3166,7 +3166,7 @@ vxge_vlan_rx_register(struct net_device *dev, struct vlan_group *grp)
>
> vxge_debug_entryexit(VXGE_TRACE, "%s:%d", __func__, __LINE__);
>
> - vdev = (struct vxgedev *)netdev_priv(dev);
> + vdev = netdev_priv(dev);
>
> vpath = &vdev->vpaths[0];
> if ((NULL == grp) && (vpath->is_open)) {
> @@ -3215,7 +3215,7 @@ vxge_vlan_rx_add_vid(struct net_device *dev, unsigned short vid)
> struct vxge_vpath *vpath;
> int vp_id;
>
> - vdev = (struct vxgedev *)netdev_priv(dev);
> + vdev = netdev_priv(dev);
>
> /* Add these vlan to the vid table */
> for (vp_id = 0; vp_id < vdev->no_of_vpath; vp_id++) {
> @@ -3242,7 +3242,7 @@ vxge_vlan_rx_kill_vid(struct net_device *dev, unsigned short vid)
>
> vxge_debug_entryexit(VXGE_TRACE, "%s:%d", __func__, __LINE__);
>
> - vdev = (struct vxgedev *)netdev_priv(dev);
> + vdev = netdev_priv(dev);
>
> vlan_group_set_device(vdev->vlgrp, vid, NULL);
>
> @@ -3475,7 +3475,7 @@ vxge_callback_crit_err(struct __vxge_hw_device *hldev,
> enum vxge_hw_event type, u64 vp_id)
> {
> struct net_device *dev = hldev->ndev;
> - struct vxgedev *vdev = (struct vxgedev *)netdev_priv(dev);
> + struct vxgedev *vdev = netdev_priv(dev);
> struct vxge_vpath *vpath = NULL;
> int vpath_idx;
>
> --
> 1.7.3.1.g432b3.dirty
>
^ permalink raw reply
* Re: [PATCH 07/11] drivers/net/vxge/vxge-main.c: Remove unnecessary casts of pci_get_drvdata
From: Jon Mason @ 2010-11-15 22:50 UTC (permalink / raw)
To: Joe Perches
Cc: Jiri Kosina, Ramkrishna Vepa, Sivakumar Subramani,
Sreenivasa Honnur, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <41a52736f96be0f61003a830b6e23e00f9a5d53b.1289851770.git.joe@perches.com>
On Mon, Nov 15, 2010 at 12:13:58PM -0800, Joe Perches wrote:
> Signed-off-by: Joe Perches <joe@perches.com>
Looks good to me.
Acked-by: Jon Mason <jon.mason@exar.com>
> ---
> drivers/net/vxge/vxge-main.c | 28 ++++++++++++----------------
> 1 files changed, 12 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/vxge/vxge-main.c b/drivers/net/vxge/vxge-main.c
> index 3f2d6ed..22c7d79 100644
> --- a/drivers/net/vxge/vxge-main.c
> +++ b/drivers/net/vxge/vxge-main.c
> @@ -688,7 +688,7 @@ static int vxge_learn_mac(struct vxgedev *vdev, u8 *mac_header)
> struct vxge_vpath *vpath = NULL;
> struct __vxge_hw_device *hldev;
>
> - hldev = (struct __vxge_hw_device *)pci_get_drvdata(vdev->pdev);
> + hldev = pci_get_drvdata(vdev->pdev);
>
> mac_address = (u8 *)&mac_addr;
> memcpy(mac_address, mac_header, ETH_ALEN);
> @@ -1313,7 +1313,7 @@ static void vxge_vpath_intr_disable(struct vxgedev *vdev, int vp_id)
> struct __vxge_hw_device *hldev;
> int msix_id;
>
> - hldev = (struct __vxge_hw_device *)pci_get_drvdata(vdev->pdev);
> + hldev = pci_get_drvdata(vdev->pdev);
>
> vxge_hw_vpath_wait_receive_idle(hldev, vpath->device_id);
>
> @@ -1632,8 +1632,7 @@ static int vxge_poll_inta(struct napi_struct *napi, int budget)
> int budget_org = budget;
> struct vxge_ring *ring;
>
> - struct __vxge_hw_device *hldev = (struct __vxge_hw_device *)
> - pci_get_drvdata(vdev->pdev);
> + struct __vxge_hw_device *hldev = pci_get_drvdata(vdev->pdev);
>
> for (i = 0; i < vdev->no_of_vpath; i++) {
> ring = &vdev->vpaths[i].ring;
> @@ -1673,7 +1672,7 @@ static void vxge_netpoll(struct net_device *dev)
> struct vxgedev *vdev;
>
> vdev = (struct vxgedev *)netdev_priv(dev);
> - hldev = (struct __vxge_hw_device *)pci_get_drvdata(vdev->pdev);
> + hldev = pci_get_drvdata(vdev->pdev);
>
> vxge_debug_entryexit(VXGE_TRACE, "%s:%d", __func__, __LINE__);
>
> @@ -2107,7 +2106,7 @@ static irqreturn_t vxge_isr_napi(int irq, void *dev_id)
> vxge_debug_intr(VXGE_TRACE, "%s:%d", __func__, __LINE__);
>
> dev = vdev->ndev;
> - hldev = (struct __vxge_hw_device *)pci_get_drvdata(vdev->pdev);
> + hldev = pci_get_drvdata(vdev->pdev);
>
> if (pci_channel_offline(vdev->pdev))
> return IRQ_NONE;
> @@ -2342,7 +2341,7 @@ static void vxge_rem_msix_isr(struct vxgedev *vdev)
> static void vxge_rem_isr(struct vxgedev *vdev)
> {
> struct __vxge_hw_device *hldev;
> - hldev = (struct __vxge_hw_device *)pci_get_drvdata(vdev->pdev);
> + hldev = pci_get_drvdata(vdev->pdev);
>
> #ifdef CONFIG_PCI_MSI
> if (vdev->config.intr_type == MSI_X) {
> @@ -2583,7 +2582,7 @@ vxge_open(struct net_device *dev)
> "%s: %s:%d", dev->name, __func__, __LINE__);
>
> vdev = (struct vxgedev *)netdev_priv(dev);
> - hldev = (struct __vxge_hw_device *)pci_get_drvdata(vdev->pdev);
> + hldev = pci_get_drvdata(vdev->pdev);
> function_mode = vdev->config.device_hw_info.function_mode;
>
> /* make sure you have link off by default every time Nic is
> @@ -2811,7 +2810,7 @@ static int do_vxge_close(struct net_device *dev, int do_io)
> dev->name, __func__, __LINE__);
>
> vdev = (struct vxgedev *)netdev_priv(dev);
> - hldev = (struct __vxge_hw_device *)pci_get_drvdata(vdev->pdev);
> + hldev = pci_get_drvdata(vdev->pdev);
>
> if (unlikely(!is_vxge_card_up(vdev)))
> return 0;
> @@ -3985,8 +3984,7 @@ static int vxge_pm_resume(struct pci_dev *pdev)
> static pci_ers_result_t vxge_io_error_detected(struct pci_dev *pdev,
> pci_channel_state_t state)
> {
> - struct __vxge_hw_device *hldev =
> - (struct __vxge_hw_device *)pci_get_drvdata(pdev);
> + struct __vxge_hw_device *hldev = pci_get_drvdata(pdev);
> struct net_device *netdev = hldev->ndev;
>
> netif_device_detach(netdev);
> @@ -4015,8 +4013,7 @@ static pci_ers_result_t vxge_io_error_detected(struct pci_dev *pdev,
> */
> static pci_ers_result_t vxge_io_slot_reset(struct pci_dev *pdev)
> {
> - struct __vxge_hw_device *hldev =
> - (struct __vxge_hw_device *)pci_get_drvdata(pdev);
> + struct __vxge_hw_device *hldev = pci_get_drvdata(pdev);
> struct net_device *netdev = hldev->ndev;
>
> struct vxgedev *vdev = netdev_priv(netdev);
> @@ -4041,8 +4038,7 @@ static pci_ers_result_t vxge_io_slot_reset(struct pci_dev *pdev)
> */
> static void vxge_io_resume(struct pci_dev *pdev)
> {
> - struct __vxge_hw_device *hldev =
> - (struct __vxge_hw_device *)pci_get_drvdata(pdev);
> + struct __vxge_hw_device *hldev = pci_get_drvdata(pdev);
> struct net_device *netdev = hldev->ndev;
>
> if (netif_running(netdev)) {
> @@ -4689,7 +4685,7 @@ static void __devexit vxge_remove(struct pci_dev *pdev)
> struct net_device *dev;
> int i = 0;
>
> - hldev = (struct __vxge_hw_device *)pci_get_drvdata(pdev);
> + hldev = pci_get_drvdata(pdev);
>
> if (hldev == NULL)
> return;
> --
> 1.7.3.1.g432b3.dirty
>
^ permalink raw reply
* Re: the future of ethtool
From: Jeff Garzik @ 2010-11-15 22:49 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Stephen Hemminger, NetDev, David Miller
In-Reply-To: <1289857936.2586.51.camel@bwh-desktop>
On 11/15/2010 04:52 PM, Ben Hutchings wrote:
> On Mon, 2010-11-15 at 13:14 -0800, Stephen Hemminger wrote:
>> On Mon, 15 Nov 2010 21:14:02 +0000
>> Ben Hutchings<bhutchings@solarflare.com> wrote:
>>
>>> On Mon, 2010-11-15 at 12:44 -0800, Stephen Hemminger wrote:
>>> [...]
>>>> My views are simple:
>>>>
>>>> Ethtool needs to be an extension of existing netlink API for interfaces.
>>>> - handles multiple values per transaction
>>>> - extensible
>>> [...]
>>>
>>> Are you suggesting to send and receive the existing ethtool command and
>>> result structures (with some wrapping) through netlink? Or some larger
>>> change to the API?
>>
>> The existing ioctl base API should be kept as legacy and something
>> better developed.
>
> So you're saying: expose all new operations through netlink (and only
> netlink) while keeping the old operations exposed only through ioctl?
> That's hardly an improvement as it means ethtool or any other
> configuration utility will have to support both APIs indefinitely.
s/only// I don't think Stephen is suggesting sending _some_ ops
through netlink and others through old-ioctl. That's just silly. Any
new netlink interface should transit all existing ETHTOOL_xxx
commands/structures.
But presumably, one would have the ability to send multiple ETHTOOL_xxx
bundled together into a single netlink transaction, facilitating the
kernel's calling of struct ethtool_ops'
->begin()
... first operation specified by userspace via netlink ...
... second operation specified by userspace via netlink ...
... etc.
->end()
The underlying struct ethtool_ops remains unchanged; you're only
changing the transit method.
Thus, the ethtool userspace utility would switch entirely to netlink,
while the ioctl processing code remains for binary compatibility.
Or... ethtool userspace utility could remain unchanged, and a new
'nictool' utility provides the same features but with (a) a new CLI and
(b) exclusively uses netlink rather than ioctl.
Jeff
^ permalink raw reply
* Re: [PATCH/RFC] netfilter: nf_conntrack_sip: Handle quirky Cisco phones
From: Kevin Cernekee @ 2010-11-15 22:09 UTC (permalink / raw)
To: Patrick McHardy
Cc: Eric Dumazet, David S. Miller, Alexey Kuznetsov,
Pekka Savola (ipv6), James Morris, Hideaki YOSHIFUJI,
netfilter-devel, netfilter, coreteam, linux-kernel, netdev
In-Reply-To: <4CE16699.1050604@trash.net>
[-- Attachment #1: Type: text/plain, Size: 603 bytes --]
On Mon, Nov 15, 2010 at 8:58 AM, Patrick McHardy <kaber@trash.net> wrote:
> Could you provide a binary tcpdump (-w file -s0) of registration
> and a subsequent call please?
These were captured running my "v2" patch, since the phone cannot
register without it:
http://lkml.org/lkml/2010/11/14/181
goodreg.pcap - good registration
goodcall.pcap - call initiation + termination
This is all that happens in the absence of the "Cisco patch":
badreg.pcap - bad registration
Phone's LAN IP is 192.168.0.28
Public IP (as seen by the SIP proxy) is 111.222.33.222
SIP proxy is sip.iptel.org = 213.192.59.75
[-- Attachment #2: goodreg.pcap --]
[-- Type: application/octet-stream, Size: 3328 bytes --]
[-- Attachment #3: goodcall.pcap --]
[-- Type: application/octet-stream, Size: 7771 bytes --]
[-- Attachment #4: badreg.pcap --]
[-- Type: application/octet-stream, Size: 13596 bytes --]
^ permalink raw reply
* Re: [PATCH] net: use the macros defined for the members of flowi
From: Eric Dumazet @ 2010-11-15 21:57 UTC (permalink / raw)
To: Brian Haley; +Cc: Changli Gao, David S. Miller, netdev
In-Reply-To: <1289857126.3364.14.camel@edumazet-laptop>
Le lundi 15 novembre 2010 à 22:38 +0100, Eric Dumazet a écrit :
> Same question on lkml few hours ago. I think gcc does the assignement to
> zero, even on automatic variables (at least done on x86), but could not
> find a doc on it.
Oh well, for sure fields that are not mentioned are set to 0, thanks to
C99.
My question on lkml was about padding holes (security related)
^ 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