Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v2 1/2] virtio-net: Verify page list size before fitting into  skb
From: Sasha Levin @ 2011-10-05 13:50 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel, Rusty Russell, virtualization, netdev, kvm
In-Reply-To: <20111003190416.GC22427@redhat.com>

On Mon, 2011-10-03 at 21:04 +0200, Michael S. Tsirkin wrote:
> On Wed, Sep 28, 2011 at 05:40:54PM +0300, Sasha Levin wrote:
> > This patch verifies that the length of a buffer stored in a linked list
> > of pages is small enough to fit into a skb.
> > 
> > If the size is larger than a max size of a skb, it means that we shouldn't
> > go ahead building skbs anyway since we won't be able to send the buffer as
> > the user requested.
> > 
> > Cc: Rusty Russell <rusty@rustcorp.com.au>
> > Cc: "Michael S. Tsirkin" <mst@redhat.com>
> > Cc: virtualization@lists.linux-foundation.org
> > Cc: netdev@vger.kernel.org
> > Cc: kvm@vger.kernel.org
> > Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> > ---
> >  drivers/net/virtio_net.c |   13 +++++++++++++
> >  1 files changed, 13 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 0c7321c..bde0dec 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -195,6 +195,19 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> >  	len -= copy;
> >  	offset += copy;
> >  
> > +	/*
> > +	 * Verify that we can indeed put this data into a skb.
> > +	 * This is here to handle cases when the device erroneously
> > +	 * tries to receive more than is possible. This is usually
> > +	 * the case of a broken device.
> > +	 */
> > +	if (unlikely(len > MAX_SKB_FRAGS * PAGE_SIZE)) {
> > +		if (net_ratelimit())
> > +			pr_debug("%s: too much data\n", skb->dev->name);
> > +		dev_kfree_skb(skb);
> > +		return NULL;
> > +	}
> > +
> 
> BTW, receive_mergeable does
>                         pr_debug("%s: packet too long\n", skb->dev->name);
>                         skb->dev->stats.rx_length_errors++;
> 
> which makes sense.

Do you think we should increase rx_length_errors here as well?

-- 

Sasha.

^ permalink raw reply

* Re: big picture UDP/IP performance question re 2.6.18  -> 2.6.32
From: Peter Zijlstra @ 2011-10-05 13:22 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: starlight, Eric Dumazet, linux-kernel, netdev, Willy Tarreau,
	Ingo Molnar
In-Reply-To: <alpine.DEB.2.00.1110041358500.12199@router.home>

On Tue, 2011-10-04 at 14:16 -0500, Christoph Lameter wrote:
> 
> We had similar experiences. Basically latency constantly gets screwed up
> by the new fancy features being added to the scheduler and network
> subsystem (most notorious is the new "fair" scheduler, 2.6.23 made a big
> step down). The kernel has a fairly constant regression in terms of
> latency release after release. Only the new and more efficient processors
> periodically provide some compensation (and some isolated patches to
> actually improve things get in but these are usually watered down one or
> two releases after those improvements have been made). 

I suppose all this testing and feedback we receive from you really helps
us keep the performance levels you want.. Oh wait, that's 0.

Clearly none of the tests being ran on a regular basis, by for instance
the Intel regression team, covers your needs. Start by fixing that.

Also, for latency, we've got ftrace and a latencytracer, provide traces
that illustrate your fail.

^ permalink raw reply

* netfilter: Use proper rwlock init function
From: Thomas Gleixner @ 2011-10-05 13:24 UTC (permalink / raw)
  To: netdev; +Cc: David Miller

Replace the open coded initialization with the init function.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 net/netfilter/ipvs/ip_vs_ctl.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6/net/netfilter/ipvs/ip_vs_ctl.c
===================================================================
--- linux-2.6.orig/net/netfilter/ipvs/ip_vs_ctl.c
+++ linux-2.6/net/netfilter/ipvs/ip_vs_ctl.c
@@ -3679,7 +3679,7 @@ int __net_init __ip_vs_control_init(stru
 	int idx;
 	struct netns_ipvs *ipvs = net_ipvs(net);
 
-	ipvs->rs_lock = __RW_LOCK_UNLOCKED(ipvs->rs_lock);
+	rwlock_init(&ipvs->rs_lock);
 
 	/* Initialize rs_table */
 	for (idx = 0; idx < IP_VS_RTAB_SIZE; idx++)

^ permalink raw reply

* Re: xfrm warning by mip6d
From: András Takács @ 2011-10-05 13:32 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1317810803.2473.37.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>

Thanks for the help. I added this two skb_dst_force calls to my kernel source, but unfortunately it didn't solved the problem.

I know it's just a warning message, but I would be happy, if I could fix it.

Does anybody have any other idea?


Thank you very much!

Regards,
András Takács
http://www.wakoond.hu

On 2011.10.05., at 12:33, Eric Dumazet wrote:

> Le mercredi 05 octobre 2011 à 11:22 +0200, András Takács a écrit :
> 
> Issue solved by commit 3bc07321ccc236f693ce1b6a8786f0a2e38bb87e
> 
> Thanks
> 
> 
> commit 3bc07321ccc236f693ce1b6a8786f0a2e38bb87e
> Author: Steffen Klassert <steffen.klassert@secunet.com>
> Date:   Tue Mar 15 21:08:28 2011 +0000
> 
>    xfrm: Force a dst refcount before entering the xfrm type handlers
> 
>    Crypto requests might return asynchronous. In this case we leave
>    the rcu protected region, so force a refcount on the skb's
>    destination entry before we enter the xfrm type input/output
>    handlers.
> 
>    This fixes a crash when a route is deleted whilst sending IPsec
>    data that is transformed by an asynchronous algorithm.
> 
>    Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
>    Signed-off-by: David S. Miller <davem@davemloft.net>
> 
> diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c
> index 872065c..341cd11 100644
> --- a/net/xfrm/xfrm_input.c
> +++ b/net/xfrm/xfrm_input.c
> @@ -190,6 +190,8 @@ int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type)
> 		XFRM_SKB_CB(skb)->seq.input.low = seq;
> 		XFRM_SKB_CB(skb)->seq.input.hi = seq_hi;
> 
> +		skb_dst_force(skb);
> +
> 		nexthdr = x->type->input(x, skb);
> 
> 		if (nexthdr == -EINPROGRESS)
> diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
> index 1aba03f..8f3f0ee 100644
> --- a/net/xfrm/xfrm_output.c
> +++ b/net/xfrm/xfrm_output.c
> @@ -78,6 +78,8 @@ static int xfrm_output_one(struct sk_buff *skb, int err)
> 
> 		spin_unlock_bh(&x->lock);
> 
> +		skb_dst_force(skb);
> +
> 		err = x->type->output(x, skb);
> 		if (err == -EINPROGRESS)
> 			goto out_exit;
> 
> 
> 

^ permalink raw reply

* Re: [PATCH] SELinux: Fix RCU deref check warning in sel_netport_insert()
From: David Howells @ 2011-10-05 13:32 UTC (permalink / raw)
  To: Paul Moore; +Cc: dhowells, selinux, netdev
In-Reply-To: <2230709.7n5noARWFd@sifl>

Paul Moore <paul@paul-moore.com> wrote:

> We should probably do the same for the security/selinux/netif.c as it uses
> the same logic; David is this something you want to tackle?

netif.c doesn't use any rcu_dereference*() function directly, though it does
use list_for_each_entry_rcu().  However, I'm not sure that's a problem.  What
is it you're referring to?

David

^ permalink raw reply

* Re: [PATCH v2 2/2] virtio-net: Prevent NULL dereference
From: Sasha Levin @ 2011-10-05 13:50 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel, Rusty Russell, virtualization, netdev, kvm
In-Reply-To: <20111003184037.GA22427@redhat.com>

On Mon, 2011-10-03 at 20:40 +0200, Michael S. Tsirkin wrote:
> On Wed, Sep 28, 2011 at 05:40:55PM +0300, Sasha Levin wrote:
> > This patch prevents a NULL dereference when the user has passed a length
> > longer than an actual buffer to virtio-net.
> > 
> > Cc: Rusty Russell <rusty@rustcorp.com.au>
> > Cc: "Michael S. Tsirkin" <mst@redhat.com>
> > Cc: virtualization@lists.linux-foundation.org
> > Cc: netdev@vger.kernel.org
> > Cc: kvm@vger.kernel.org
> > Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> > ---
> >  drivers/net/virtio_net.c |   12 +++++++++++-
> >  1 files changed, 11 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index bde0dec..4a53d2a 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -208,12 +208,22 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> >  		return NULL;
> >  	}
> >  
> > -	while (len) {
> > +	while (len && page) {
> >  		set_skb_frag(skb, page, offset, &len);
> >  		page = (struct page *)page->private;
> >  		offset = 0;
> >  	}
> >  
> > +	/*
> > +	 * This is the case where we ran out of pages in our linked list, but
> > +	 * supposedly have more data to read.
> 
> Again, let's clarify that this only happens with broken devices.

I think that the code within the if() makes it clear that it isn't the
regular path.

-- 

Sasha.

^ permalink raw reply

* RE: netfilter: Use proper rwlock init function
From: Hans Schillström @ 2011-10-05 13:54 UTC (permalink / raw)
  To: Thomas Gleixner, netdev@vger.kernel.org, Julian Anastasov,
	horms@verge.net.au
  Cc: David Miller
In-Reply-To: <alpine.LFD.2.02.1110051522410.18778@ionos>

Hello Simon,
you can drop my "fix lockdep warning" patch
since Thomas patch is identical to what I prepared to send...

>Replace the open coded initialization with the init function.
>
>Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

Ack-by: Hans Schillstrom <hans.schillstrom@ericsson.com>

>---
> net/netfilter/ipvs/ip_vs_ctl.c |    2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>Index: linux-2.6/net/netfilter/ipvs/ip_vs_ctl.c
>===================================================================
>--- linux-2.6.orig/net/netfilter/ipvs/ip_vs_ctl.c
>+++ linux-2.6/net/netfilter/ipvs/ip_vs_ctl.c
>@@ -3679,7 +3679,7 @@ int __net_init __ip_vs_control_init(stru
>        int idx;
>        struct netns_ipvs *ipvs = net_ipvs(net);
>
>-       ipvs->rs_lock = __RW_LOCK_UNLOCKED(ipvs->rs_lock);
>+       rwlock_init(&ipvs->rs_lock);
>
>        /* Initialize rs_table */
>        for (idx = 0; idx < IP_VS_RTAB_SIZE; idx++)

Thanks
Hans

^ permalink raw reply

* Re: xfrm warning by mip6d
From: Eric Dumazet @ 2011-10-05 13:55 UTC (permalink / raw)
  To: András Takács; +Cc: netdev
In-Reply-To: <34287764-C788-4718-BDD8-8B4DC98FE9F4@wakoond.hu>

Le mercredi 05 octobre 2011 à 15:32 +0200, András Takács a écrit :
> Thanks for the help. I added this two skb_dst_force calls to my kernel source, but unfortunately it didn't solved the problem.
> 
> I know it's just a warning message, but I would be happy, if I could fix it.
> 
> Does anybody have any other idea?

Is a more recent kernel issuing the same error ?

^ permalink raw reply

* Re: [PATCH] net-proc: expose the tos values in /proc/net/[tcp|udp]
From: MuraliRaja Muniraju @ 2011-10-05 14:09 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, kuznet, jmorris, yoshfuji, kaber, netdev,
	linux-kernel
In-Reply-To: <1317807270.2473.35.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>

Hi Eric,
     I am trying one more approach of adding a new attribute to the
inet_diag extension to expose the tos values. Hence with this approach
tcp tos problem would be solved. When  udp would also be supported via
netlink and will be using inet_diag, it would be just be publishing
the tos values in the new extension added. Can you let me know your
thoughts on the following patch.

--- a/include/linux/inet_diag.h
+++ b/include/linux/inet_diag.h
@@ -97,9 +97,10 @@ enum {
        INET_DIAG_INFO,
        INET_DIAG_VEGASINFO,
        INET_DIAG_CONG,
+       INET_DIAG_TOS,
 };

-#define INET_DIAG_MAX INET_DIAG_CONG
+#define INET_DIAG_MAX INET_DIAG_TOS


 /* INET_DIAG_MEM */
@@ -120,6 +121,13 @@ struct tcpvegas_info {
        __u32   tcpv_minrtt;
 };

+/* INET_DIAG_TOS */
+
+struct inet_diag_tos {
+       __u8    idiag_tos;
+       __u8    idiag_reserved[3];
+};
+
 #ifdef __KERNEL__
 struct sock;
 struct inet_hashinfo;
diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
index e5fa2dd..abdd606 100644
--- a/net/ipv4/inet_diag.c
+++ b/net/ipv4/inet_diag.c
@@ -82,6 +82,7 @@ static int inet_csk_diag_fill(struct sock *sk,
        struct nlmsghdr  *nlh;
        void *info = NULL;
        struct inet_diag_meminfo  *minfo = NULL;
+       struct inet_diag_tos *tos = NULL;
        unsigned char    *b = skb_tail_pointer(skb);
        const struct inet_diag_handler *handler;

@@ -108,6 +109,9 @@ static int inet_csk_diag_fill(struct sock *sk,
                       icsk->icsk_ca_ops->name);
        }

+       if (ext & (1 << (INET_DIAG_TOS - 1)))
+               tos = INET_DIAG_PUT(skb, INET_DIAG_TOS, sizeof(*tos));
+
        r->idiag_family = sk->sk_family;
        r->idiag_state = sk->sk_state;
        r->idiag_timer = 0;
@@ -169,6 +173,9 @@ static int inet_csk_diag_fill(struct sock *sk,
            icsk->icsk_ca_ops && icsk->icsk_ca_ops->get_info)
                icsk->icsk_ca_ops->get_info(sk, ext, skb);

+       if (tos)
+               tos->idiag_tos = inet->tos;
+
        nlh->nlmsg_len = skb_tail_pointer(skb) - b;
        return skb->len;


Thanks,
Murali

On Wed, Oct 5, 2011 at 2:34 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le mardi 04 octobre 2011 à 10:38 -0700, MuraliRaja Muniraju a écrit :
>> I shall make the changes by exposing the tos values via the
>> netlink as suggested. I had a doubt is I have to modify the
>> inet_diag_sockid or export it via a new option. I am considering
>> that adding into inet_diag_sockid is better because the latter is a
>> bit of a over kill for a single value to be exposed and it also seems
>> to be logical fit.
>> Can you let me know your thoughts on this.
>
> I believe you could add one "u32 tos" to struct tcp_info
>
> [ It wont solve the UDP case, since "ss -u" still dumps /proc/net/udp
> and /proc/net/udp6 ]
>
> Following patch should handle tcp/dccp ipv4/ipv6
>
> diff --git a/include/linux/tcp.h b/include/linux/tcp.h
> index 7f59ee9..eec6f3b 100644
> --- a/include/linux/tcp.h
> +++ b/include/linux/tcp.h
> @@ -167,6 +167,7 @@ struct tcp_info {
>        __u32   tcpi_rcv_space;
>
>        __u32   tcpi_total_retrans;
> +       __u32   tos;
>  };
>
>  /* for TCP_MD5SIG socket option */
> diff --git a/net/dccp/diag.c b/net/dccp/diag.c
> index b21f261..70d9498 100644
> --- a/net/dccp/diag.c
> +++ b/net/dccp/diag.c
> @@ -37,6 +37,7 @@ static void dccp_get_info(struct sock *sk, struct tcp_info *info)
>
>        if (dp->dccps_hc_tx_ccid != NULL)
>                ccid_hc_tx_get_info(dp->dccps_hc_tx_ccid, sk, info);
> +       info->tos = inet_sk(sk)->tos;
>  }
>
>  static void dccp_diag_get_info(struct sock *sk, struct inet_diag_msg *r,
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 4c0da24..b24c3d8 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -2493,6 +2493,7 @@ void tcp_get_info(struct sock *sk, struct tcp_info *info)
>        info->tcpi_rcv_space = tp->rcvq_space.space;
>
>        info->tcpi_total_retrans = tp->total_retrans;
> +       info->tos = inet_sk(sk)->tos;
>  }
>  EXPORT_SYMBOL_GPL(tcp_get_info);
>
>
>
>



-- 
Thanks,
Murali

^ permalink raw reply related

* [PATCH] route: fix ICMP redirect validation
From: Flavio Leitner @ 2011-10-05 14:20 UTC (permalink / raw)
  To: netdev; +Cc: David Miller, Flavio Leitner

The commit f39925dbde7788cfb96419c0f092b086aa325c0f
(ipv4: Cache learned redirect information in inetpeer.)
removed some ICMP packet validations which are required by
RFC 1122, section 3.2.2.2:
...
  A Redirect message SHOULD be silently discarded if the new
  gateway address it specifies is not on the same connected
  (sub-) net through which the Redirect arrived [INTRO:2,
  Appendix A], or if the source of the Redirect is not the
  current first-hop gateway for the specified destination (see
  Section 3.3.1).

Signed-off-by: Flavio Leitner <fbl@redhat.com>
---
 net/ipv4/route.c |   47 ++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 26c77e1..1c11f28 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1308,7 +1308,12 @@ static void rt_del(unsigned hash, struct rtable *rt)
 void ip_rt_redirect(__be32 old_gw, __be32 daddr, __be32 new_gw,
 		    __be32 saddr, struct net_device *dev)
 {
+	int s, i;
 	struct in_device *in_dev = __in_dev_get_rcu(dev);
+	struct rtable *rth;
+	struct rtable __rcu **rthp;
+	__be32 skeys[2] = { saddr, 0 };
+	int    ikeys[2] = { dev->ifindex, 0 };
 	struct inet_peer *peer;
 	struct net *net;
 
@@ -1321,6 +1326,9 @@ void ip_rt_redirect(__be32 old_gw, __be32 daddr, __be32 new_gw,
 	    ipv4_is_zeronet(new_gw))
 		goto reject_redirect;
 
+	if (!rt_caching(net))
+		goto reject_redirect;;
+
 	if (!IN_DEV_SHARED_MEDIA(in_dev)) {
 		if (!inet_addr_onlink(in_dev, new_gw, old_gw))
 			goto reject_redirect;
@@ -1331,13 +1339,42 @@ void ip_rt_redirect(__be32 old_gw, __be32 daddr, __be32 new_gw,
 			goto reject_redirect;
 	}
 
-	peer = inet_getpeer_v4(daddr, 1);
-	if (peer) {
-		peer->redirect_learned.a4 = new_gw;
+	for (s = 0; s < 2; s++) {
+		for (i = 0; i < 2; i++) {
+			unsigned int hash = rt_hash(daddr, skeys[s],
+						    ikeys[i], rt_genid(net));
 
-		inet_putpeer(peer);
+			rthp=&rt_hash_table[hash].chain;
 
-		atomic_inc(&__rt_peer_genid);
+			while ((rth = rcu_dereference(*rthp)) != NULL) {
+
+				if (rth->rt_key_dst != daddr ||
+				    rth->rt_key_src != skeys[s] ||
+				    rth->rt_oif != ikeys[i] ||
+				    rt_is_input_route(rth) ||
+				    rt_is_expired(rth) ||
+				    !net_eq(dev_net(rth->dst.dev), net)) {
+					rthp = &rth->dst.rt_next;
+					continue;
+				}
+
+				if (rth->rt_dst != daddr ||
+				    rth->rt_src != saddr ||
+				    rth->rt_gateway != old_gw ||
+				    rth->dst.dev != dev ||
+				    rth->dst.error)
+					break;
+
+				peer = inet_getpeer_v4(daddr, 1);
+				if (peer) {
+					peer->redirect_learned.a4 = new_gw;
+					inet_putpeer(peer);
+					atomic_inc(&__rt_peer_genid);
+				}
+
+				break;
+			}
+		}
 	}
 	return;
 
-- 
1.7.6

^ permalink raw reply related

* Re: big picture UDP/IP performance question re 2.6.18  -> 2.6.32
From: Christoph Lameter @ 2011-10-05 14:26 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: starlight, Eric Dumazet, linux-kernel, netdev, Willy Tarreau,
	Ingo Molnar
In-Reply-To: <1317820942.6766.26.camel@twins>

On Wed, 5 Oct 2011, Peter Zijlstra wrote:

> Clearly none of the tests being ran on a regular basis, by for instance
> the Intel regression team, covers your needs. Start by fixing that.

The most commonly run, the aged old tests contained in the AIM9 suite,
have consistently shown these regressions over long years and they have
been brought up repeatedly in numerous discussions. This is pervasive
thoughout the OS hotpaths. Just look at how the page fault latencies
change over time. Take a modern machine and then run successively older
kernel versions on it. You will see performance getting better and
latencies becoming smaller.

> Also, for latency, we've got ftrace and a latencytracer, provide traces
> that illustrate your fail.

We would need a backport of both to a kernel version that works with
reasonable latencies so that we can figure out what caused these
regressions for this particular case. Disabling network and kernel
features usually gives you better performance but there are a lot of
things in the hot paths these days that can not be disabled.

^ permalink raw reply

* Re: big picture UDP/IP performance question re 2.6.18 -> 2.6.32
From: starlight @ 2011-10-05 15:12 UTC (permalink / raw)
  To: Peter Zijlstra, Christoph Lameter
  Cc: Eric Dumazet, linux-kernel, netdev, Willy Tarreau, Ingo Molnar
In-Reply-To: <1317820942.6766.26.camel@twins>

At 03:22 PM 10/5/2011 +0200, Peter Zijlstra wrote:
>On Tue, 2011-10-04 at 14:16 -0500, Christoph Lameter wrote:
>I suppose all this testing and feedback we receive
>from you really helps us keep the performance
>levels you want.. Oh wait, that's 0.

Well I have to admit that I've looked into the
future (i.e. tested kernel.org releases) only on
occasion.  To the extent that performance wasn't
looking great I harbored an apparently over
optimistic hope that things would get tuned/fixed
in the process somewhere.  In particular I was
thinking that RH might do something impressive
with RHEL 6 where Novell failed with SLES 11.
However, if anything the downstream is fiddling
less and less with the kernel as it becomes ever
more complex.

>Clearly none of the tests being ran on a regular
>basis, by for instance the Intel regression team,
>covers your needs. Start by fixing that.

I could create a test case.  It's quite a lot more
than trivial, but not ridiculous either.  That UDP
is broken on everything past 2.6.27 may goad me
into the effort.  Have had a fairly excellent
experience WRT the hugepage kernel corruption bugs
I reported 18 months or so ago.  Not only did the
kernel developers fix them, the fixes made it
downstream into RHEL fairly quickly.  So that does
encourage me.

^ permalink raw reply

* Re: big picture UDP/IP performance question re 2.6.18  -> 2.6.32
From: Andi Kleen @ 2011-10-05 15:12 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: Peter Zijlstra, starlight, Eric Dumazet, linux-kernel, netdev,
	Willy Tarreau, Ingo Molnar
In-Reply-To: <alpine.DEB.2.00.1110050915160.30467@router.home>

Christoph Lameter <cl@gentwo.org> writes:

> On Wed, 5 Oct 2011, Peter Zijlstra wrote:
>
>> Clearly none of the tests being ran on a regular basis, by for instance
>> the Intel regression team, covers your needs. Start by fixing that.
>
> The most commonly run, the aged old tests contained in the AIM9 suite,
> have consistently shown these regressions over long years and they have
> been brought up repeatedly in numerous discussions. This is pervasive
> thoughout the OS hotpaths. Just look at how the page fault latencies
> change over time. Take a modern machine and then run successively older
> kernel versions on it. You will see performance getting better and
> latencies becoming smaller.

One of the reason the Intel tests fix problems is that Alex and Tim
and others look into them and track down regressions. If you see
a new problem during your testing you should do the same.
This actually helps.

It doesn't need to be a full analysis/patch, even just posting some detailed
information on a new regression is useful.

For example one tool I found useful is to just enable the function
graph tracer and look at the latencies of functions in that path
during the test. 

If something changes dramatically it's relatively easy to point to.

So for example if you see context switch changing it should not 
be that difficult to do such a trace and at least point to the guilty
functions and post a mail.

>> Also, for latency, we've got ftrace and a latencytracer, provide traces
>> that illustrate your fail.
>
> We would need a backport of both to a kernel version that works with
> reasonable latencies so that we can figure out what caused these
> regressions for this particular case. Disabling network and kernel
> features usually gives you better performance but there are a lot of
> things in the hot paths these days that can not be disabled.

Please do function traces and point fingers at slow things in hotpath.
The more the merrier. Post a list of a shame!

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only

^ permalink raw reply

* Re: big picture UDP/IP performance question re 2.6.18  -> 2.6.32
From: Peter Zijlstra @ 2011-10-05 15:33 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: starlight, Eric Dumazet, linux-kernel, netdev, Willy Tarreau,
	Ingo Molnar
In-Reply-To: <alpine.DEB.2.00.1110050915160.30467@router.home>

On Wed, 2011-10-05 at 09:26 -0500, Christoph Lameter wrote:
> 
> > Also, for latency, we've got ftrace and a latencytracer, provide traces
> > that illustrate your fail.
> 
> We would need a backport of both to a kernel version that works with
> reasonable latencies so that we can figure out what caused these
> regressions for this particular case. Disabling network and kernel
> features usually gives you better performance but there are a lot of
> things in the hot paths these days that can not be disabled.

Well can can also just take the status quo and use tools like ftrace and
perf to find out what your hot paths are and where you're spending time.

On -rt we use the (irq)latencytracer a _lot_ to find problems, a
detailed function trace of WTH the kernel thinks its doing helps a lot
with trying to fix it.

Things like cyclictest are also useful to find application level
latencies, it can trace the entire latency path if needed.

Kernel level profiles are also a great tool to find out where you're
spending your time.

^ permalink raw reply

* [PATCH net] mscan: zero accidentally copied register content
From: Oliver Hartkopp @ 2011-10-05 15:34 UTC (permalink / raw)
  To: Wolfgang Grandegger, Wolfram Sang; +Cc: Linux Netdev List, Andre Naujoks

Due to the 16 bit access to mscan registers there's too much data copied to
the zero initialized CAN frame when having an odd number of bytes to copy.
This patch clears the data byte read from the invalid register entry.

Reported-by: Andre Naujoks <nautsch@gmail.com>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>

---

Hello Wolf[gang|ram],

from an error report from Andre Naujoks i tracked down the problem of
uninitialized data in (normally) initialized CAN frames to the mscan driver.

Regards,
Oliver


diff --git a/drivers/net/can/mscan/mscan.c b/drivers/net/can/mscan/mscan.c
index 92feac6..1b60fbe 100644
--- a/drivers/net/can/mscan/mscan.c
+++ b/drivers/net/can/mscan/mscan.c
@@ -327,20 +327,23 @@ static void mscan_get_rx_frame(struct net_device *dev, struct can_frame *frame)
 	frame->can_dlc = get_can_dlc(in_8(&regs->rx.dlr) & 0xf);
 
 	if (!(frame->can_id & CAN_RTR_FLAG)) {
 		void __iomem *data = &regs->rx.dsr1_0;
 		u16 *payload = (u16 *)frame->data;
 
 		for (i = 0; i < (frame->can_dlc + 1) / 2; i++) {
 			*payload++ = in_be16(data);
 			data += 2 + _MSCAN_RESERVED_DSR_SIZE;
 		}
+		/* zero accidentally copied register content at odd DLCs */
+		if (frame->can_dlc & 1)
+			frame->data[frame->can_dlc] = 0;
 	}
 
 	out_8(&regs->canrflg, MSCAN_RXF);
 }
 
 static void mscan_get_err_frame(struct net_device *dev, struct can_frame *frame,
 				u8 canrflg)
 {
 	struct mscan_priv *priv = netdev_priv(dev);
 	struct mscan_regs *regs = (struct mscan_regs *)priv->reg_base;

^ permalink raw reply related

* Re: sky2 bridge hw csum error since 3.0.6
From: Stephen Hemminger @ 2011-10-05 15:38 UTC (permalink / raw)
  To: Tobias Powalowski; +Cc: netdev
In-Reply-To: <4E8C5D88.4020500@googlemail.com>

On Wed, 05 Oct 2011 15:37:12 +0200
Tobias Powalowski <tobias.powalowski@googlemail.com> wrote:

> Hi,
> here the log of the new 3.0.6 kernel and my sky2 ethernet card:
> eth0: hw csum failure.
> [   47.462091] Pid: 0, comm: swapper Not tainted 3.0-ARCH #1
> [   47.462094] Call Trace:
> [   47.462097]  <IRQ>  [<ffffffff81334ca2>] netdev_rx_csum_fault+0x42/0x50
> [   47.462112]  [<ffffffff8132b700>] __skb_checksum_complete_head+0x60/0x70
> [   47.462116]  [<ffffffff8132b721>] __skb_checksum_complete+0x11/0x20
> [   47.462128]  [<ffffffffa05bcfba>] br_multicast_rcv+0x32a/0x1350 [bridge]
> [   47.462134]  [<ffffffff8136758e>] ? ip_rcv+0x21e/0x2f0
> [   47.462140]  [<ffffffff8121aa96>] ? cpumask_next_and+0x36/0x50
> [   47.462145]  [<ffffffff8104a64d>] ? select_task_rq_fair+0x53d/0x9e0
> [   47.462152]  [<ffffffffa05b3fd0>] ? br_handle_local_finish+0x50/0x50
> [bridge]
> [   47.462159]  [<ffffffffa05b3fd0>] ? br_handle_local_finish+0x50/0x50
> [bridge]
> [   47.462165]  [<ffffffffa05b4087>] br_handle_frame_finish+0xb7/0x2a0
> [bridge]
> [   47.462172]  [<ffffffffa05b9e88>]
> br_nf_pre_routing_finish_ipv6+0x108/0x160 [bridge]
> [   47.462178]  [<ffffffffa05ba8dc>] br_nf_pre_routing+0x55c/0x710 [bridge]
> [   47.462183]  [<ffffffff81055b10>] ? try_to_wake_up+0x290/0x290
> [   47.462188]  [<ffffffff8135f7f5>] nf_iterate+0x85/0xc0
> [   47.462194]  [<ffffffffa05b3fd0>] ? br_handle_local_finish+0x50/0x50
> [bridge]
> [   47.462198]  [<ffffffff8135f8ae>] nf_hook_slow+0x7e/0x130
> [   47.462204]  [<ffffffffa05b3fd0>] ? br_handle_local_finish+0x50/0x50
> [bridge]
> [   47.462210]  [<ffffffff81380da1>] ? tcp_rcv_established+0x4d1/0x830
> [   47.462216]  [<ffffffffa05b4420>] br_handle_frame+0x1b0/0x270 [bridge]
> [   47.462222]  [<ffffffffa05b4270>] ?
> br_handle_frame_finish+0x2a0/0x2a0 [bridge]
> [   47.462227]  [<ffffffff8133415e>] __netif_receive_skb+0x16e/0x660
> [   47.462232]  [<ffffffff81334e2d>] netif_receive_skb+0xad/0xc0
> [   47.462236]  [<ffffffff813357a0>] napi_skb_finish+0x50/0x70
> [   47.462240]  [<ffffffff81335875>] napi_gro_receive+0xb5/0xc0
> [   47.462256]  [<ffffffffa028cce3>] sky2_poll+0x693/0xce0 [sky2]
> [   47.462260]  [<ffffffff8136758e>] ? ip_rcv+0x21e/0x2f0
> [   47.462265]  [<ffffffff81334302>] ? __netif_receive_skb+0x312/0x660
> [   47.462269]  [<ffffffff81335109>] net_rx_action+0x149/0x300
> [   47.462274]  [<ffffffff81063e40>] __do_softirq+0xb0/0x270
> [   47.462279]  [<ffffffff813f711c>] call_softirq+0x1c/0x30
> [   47.462283]  [<ffffffff8100da75>] do_softirq+0x65/0xa0
> [   47.462287]  [<ffffffff8106434e>] irq_exit+0x9e/0xc0
> [   47.462291]  [<ffffffff813f7973>] do_IRQ+0x63/0xe0
> [   47.462296]  [<ffffffff813f5653>] common_interrupt+0x13/0x13
> [   47.462298]  <EOI>  [<ffffffff81014375>] ? mwait_idle+0x95/0x2e0
> [   47.462307]  [<ffffffff8100a21a>] cpu_idle+0xba/0x100
> [   47.462311]  [<ffffffff813d31f2>] rest_init+0x96/0xa4
> [   47.462315]  [<ffffffff81748c23>] start_kernel+0x3de/0x3eb
> [   47.462319]  [<ffffffff81748347>] x86_64_start_reservations+0x132/0x136
> [   47.462323]  [<ffffffff81748140>] ? early_idt_handlers+0x140/0x140
> [   47.462327]  [<ffffffff8174844d>] x86_64_start_kernel+0x102/0x111
> 
> thanks for your help,
> kernel 3.0.4 is not affected by this.
> greetings
> tpowa

There were no changes to the sky2 driver between 3.0.4 and 3.0.6  but there were
changes to multicast tracking.  A couple of other pieces of information that might
help.  First, what is the chip type:
   dmesg | grep sky2

And could you try 'git bisect' to narrow the problem down to a particular
change.

Thanks

P.s: adding netdev list to this to make discussion public.

^ permalink raw reply

* Re: [PATCH net] mscan: zero accidentally copied register content
From: Wolfram Sang @ 2011-10-05 15:51 UTC (permalink / raw)
  To: Oliver Hartkopp; +Cc: Wolfgang Grandegger, Linux Netdev List, Andre Naujoks
In-Reply-To: <4E8C78E8.3010605@hartkopp.net>

[-- Attachment #1: Type: text/plain, Size: 1800 bytes --]

On Wed, Oct 05, 2011 at 05:34:00PM +0200, Oliver Hartkopp wrote:
> Due to the 16 bit access to mscan registers there's too much data copied to
> the zero initialized CAN frame when having an odd number of bytes to copy.
> This patch clears the data byte read from the invalid register entry.
> 
> Reported-by: Andre Naujoks <nautsch@gmail.com>
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> 
> ---
> 
> Hello Wolf[gang|ram],
> 
> from an error report from Andre Naujoks i tracked down the problem of
> uninitialized data in (normally) initialized CAN frames to the mscan driver.
> 
> Regards,
> Oliver
> 
> 
> diff --git a/drivers/net/can/mscan/mscan.c b/drivers/net/can/mscan/mscan.c
> index 92feac6..1b60fbe 100644
> --- a/drivers/net/can/mscan/mscan.c
> +++ b/drivers/net/can/mscan/mscan.c
> @@ -327,20 +327,23 @@ static void mscan_get_rx_frame(struct net_device *dev, struct can_frame *frame)
>  	frame->can_dlc = get_can_dlc(in_8(&regs->rx.dlr) & 0xf);
>  
>  	if (!(frame->can_id & CAN_RTR_FLAG)) {
>  		void __iomem *data = &regs->rx.dsr1_0;
>  		u16 *payload = (u16 *)frame->data;
>  
>  		for (i = 0; i < (frame->can_dlc + 1) / 2; i++) {
>  			*payload++ = in_be16(data);
>  			data += 2 + _MSCAN_RESERVED_DSR_SIZE;
>  		}
> +		/* zero accidentally copied register content at odd DLCs */
> +		if (frame->can_dlc & 1)
> +			frame->data[frame->can_dlc] = 0;
>  	}
>  
>  	out_8(&regs->canrflg, MSCAN_RXF);

Nice catch, but wouldn't it be more elegant to never have an invalid byte
in the first place?

if (can_dlc & 1)
	*payload = in_be16() & mask;

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* Re: [RFC] iproute2: add br command
From: Stephen Hemminger @ 2011-10-05 15:58 UTC (permalink / raw)
  To: Sridhar Samudrala; +Cc: netdev, John Fastabend
In-Reply-To: <4E8BE63F.4020706@us.ibm.com>

On Tue, 04 Oct 2011 22:08:15 -0700
Sridhar Samudrala <sri@us.ibm.com> wrote:

> 
> On Tue, 2011-10-04 at 16:00 -0700, Stephen Hemminger wrote:
> > On Tue, 04 Oct 2011 15:54:07 -0700
> > Sridhar Samudrala<sri@us.ibm.com  <mailto:sri@us.ibm.com>>  wrote:
> >
> > >  On Tue, 2011-10-04 at 15:42 -0700, Stephen Hemminger wrote:
> > >  >  On Wed, 5 Oct 2011 00:32:31 +0200
> > >  >  Michał Mirosław<mirqus@gmail.com  <mailto:mirqus@gmail.com>>  wrote:
> > >  >
> > >  >  >  2011/10/5 Sridhar Samudrala<sri@us.ibm.com  <mailto:sri@us.ibm.com>>:
> > >  >  >  >  On Tue, 2011-10-04 at 14:07 -0700, Stephen Hemminger wrote:
> > >  >  >  >>  On Tue, 04 Oct 2011 09:58:55 -0700
> > >  >  >  >>  Andi Kleen<andi@firstfloor.org  <mailto:andi@firstfloor.org>>  wrote:
> > >  >  >  >>  >  Stephen Hemminger<shemminger@vyatta.com  <mailto:shemminger@vyatta.com>>  writes:
> > >  >  >  >>  >  >  This adds a new 'br' command which is the bridging equivalent of
> > >  >  >  >>  >  >  the ip command. More of a demo of how to use netlink and bridging
> > >  >  >  >>  >  >  at this point.
> > >  >  >  >>  >  Please name it "bridge", not "br"
> > >  >  >  >>  Ok, but it breaks the existing pattern.
> > >  >  >  >  Is this supposed to replace brctl utility?
> > >  >  >  >
> > >  >  >  >  Can we add/delete a bridge and add/delete interfaces to a bridge using
> > >  >  >  >  this command?
> > >  >  >  >
> > >  >  >  >  If so, why not make it
> > >  >  >  >    ip bridge add/del<brname>
> > >  >  >  >    ip bridge addif/delif<brname>  <ifname>
> > >  >  >
> > >  >  >  I'll add one more idea:
> > >  >  >
> > >  >  >  ip link add/del<brname>  type bridge
> > >  >  >  ip bridge add/del<brname>  <ifname>
> > >  >  >  ip bridge fdb ...
> > >  >
> > >  >  In 3.0 you can already do:
> > >  >  # ip link add dev br3 type bridge
> This is a new syntax that i think works only for 'bridge' type.


The bridge interface to 'ip link' is nothing special. The same syntax
is available for macvlan, vlan, and bonding.


> All other types require a prefix 'ip link add link'



For vlan:
  ip link add link eth0 name eth0.1 type vlan id 1

The reason there is no 'link' parameter when creating a bridge is that
the bridge is a standalone device it is not slaved to an underlying
device.

> 
> I think the ip command help text should be updated to include this syntax.


patches accepted...

> 
> > >  >  # ip link set dev eth3 master br3
> 
> looks like the command to delete an interface from a bridge is
>        ip link set dev eth3 nomaster
> 
> Somehow this interface doesn't look all that intuitive.

Propose something better. 


> Currently we have 'ip route' and 'ip neigh' to manipulate routing and
> neighbor entry tables. I think 'ip bridge' would be good way to extend
> this to support bridging.
> 
> If it is not too late, can we have a interface where all bridge related
> commands can be done using the same prefix.
> 
> I think we also should think about how this syntax can be extended to show
> embedded bridges in SR-IOV NICs.

The iproute2 commands in general try to be as close to the
underlying netlink protocol. This makes it less user friendly but smaller,
easier to program. The only part of bridging that is special is the
forwarding database which is similar to the neighbor table.
If I do merge bridging into ip, it would not be as 'ip bridge' but as
another protocol family.

   ip -family bridge neigh ...


As far as SR-IOV, the interface in the kernel is netlink and the hardware
people are free to use netlink to use the same messages for SR-IOV, but
haven't.



  

^ permalink raw reply

* Re: [PATCH net] mscan: zero accidentally copied register content
From: Oliver Hartkopp @ 2011-10-05 16:10 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: Wolfgang Grandegger, Linux Netdev List, Andre Naujoks
In-Reply-To: <20111005155127.GB13794@pengutronix.de>

On 10/05/11 17:51, Wolfram Sang wrote:

> On Wed, Oct 05, 2011 at 05:34:00PM +0200, Oliver Hartkopp wrote:
>> Due to the 16 bit access to mscan registers there's too much data copied to
>> the zero initialized CAN frame when having an odd number of bytes to copy.
>> This patch clears the data byte read from the invalid register entry.
>>
>> Reported-by: Andre Naujoks <nautsch@gmail.com>
>> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
>>
>> ---
>>
>> Hello Wolf[gang|ram],
>>
>> from an error report from Andre Naujoks i tracked down the problem of
>> uninitialized data in (normally) initialized CAN frames to the mscan driver.
>>
>> Regards,
>> Oliver
>>
>>
>> diff --git a/drivers/net/can/mscan/mscan.c b/drivers/net/can/mscan/mscan.c
>> index 92feac6..1b60fbe 100644
>> --- a/drivers/net/can/mscan/mscan.c
>> +++ b/drivers/net/can/mscan/mscan.c
>> @@ -327,20 +327,23 @@ static void mscan_get_rx_frame(struct net_device *dev, struct can_frame *frame)
>>  	frame->can_dlc = get_can_dlc(in_8(&regs->rx.dlr) & 0xf);
>>  
>>  	if (!(frame->can_id & CAN_RTR_FLAG)) {
>>  		void __iomem *data = &regs->rx.dsr1_0;
>>  		u16 *payload = (u16 *)frame->data;
>>  
>>  		for (i = 0; i < (frame->can_dlc + 1) / 2; i++) {
>>  			*payload++ = in_be16(data);
>>  			data += 2 + _MSCAN_RESERVED_DSR_SIZE;
>>  		}
>> +		/* zero accidentally copied register content at odd DLCs */
>> +		if (frame->can_dlc & 1)
>> +			frame->data[frame->can_dlc] = 0;
>>  	}
>>  
>>  	out_8(&regs->canrflg, MSCAN_RXF);
> 
> Nice catch, but wouldn't it be more elegant to never have an invalid byte
> in the first place?
> 
> if (can_dlc & 1)
> 	*payload = in_be16() & mask;
> 


Hm, then i would rather think about changing the for() statement and to read
byte-by-byte instead of the current in_be16() usage with the 16bit access
drawbacks ...

Regards,
Oliver

^ permalink raw reply

* Ref: UN052-0320
From: Charity Frazier @ 2011-10-05  8:33 UTC (permalink / raw)




^ permalink raw reply

* IPv4 multicast and mac-vlans acting weird on 3.0.4+
From: Ben Greear @ 2011-10-05 16:46 UTC (permalink / raw)
  To: netdev


This is on a hacked 3.0.4 kernel...

I am seeing an issue where an IPv4 mcast receiver will not receive
a 1473 or larger byte mcast message, but will receive a 1472.  The difference
being that 1473 ends up being two packets on the wire.  It works on
802.1Q VLANs, VETH interfaces and real Ethernet.  It does not work
on a mac-vlan hanging off the VETH.

I see packets received on the macvlan in tshark, and they appear correct.  No
obvious errors in the macvlan port stats or netstat -s,
and the 'ss' tool doesn't appear to support UDP sockets at all.

So, I'm about to go digging into the code, but if anyone has any
suggestions for places to look, please let me know!

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

^ permalink raw reply

* Re: [net-next 1/1] bna: Multiple Definition and Interface Setup Fix
From: Randy Dunlap @ 2011-10-05 17:09 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: David Miller, rmody, netdev, linux-next, linux-kernel, linux-scsi,
	huangj, adapter_linux_open_src_team, Mauro Carvalho Chehab
In-Reply-To: <20111005192601.fa947a78b188f4a021042a5e@canb.auug.org.au>

On 10/05/11 01:26, Stephen Rothwell wrote:
> On Tue, 04 Oct 2011 23:27:13 -0400 (EDT) David Miller <davem@davemloft.net> wrote:
>>
>> From: Rasesh Mody <rmody@brocade.com>
>> Date: Tue, 4 Oct 2011 19:31:20 -0700
>>
>>> drivers/net/built-in.o: In function `bfa_ioc_ct2_poweron':
>>> (.text+0xcdc90): multiple definition of `bfa_ioc_ct2_poweron'
>>> drivers/scsi/built-in.o:(.text+0x17f9a0): first defined here
>>>
>>> This patch renames bfa_ioc_ct2_poweron() to bfa_nw_ioc_ct2_poweron() to avoid
>>> multiple definition with Brocade scsi driver. It also modifies asic specific
>>> interface setup to allocate MSIX resources at power on in case of 1860 HW with
>>> no asic block and warns if the asic gen is neither BFI_ASIC_GEN_CT nor
>>> BFI_ASIC_GEN_CT2.
>>>
>>> Reported-by: Randy Dunlap <rdunlap@xenotime.net>
>>> Signed-off-by: Rasesh Mody <rmody@brocade.com>
>>
>> Applied, thanks.

Thanks for the patch.

> I also manually applied this to linux-next today.

We could also stand to have this patch applied to linux-next since
it doesn't seem to be merged anywhere else yet: (for drivers/media/video)

http://marc.info/?l=linux-next&m=131741851601754&w=2


-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

^ permalink raw reply

* [PATCH net-next] Add ethtool -g support to 8139cp
From: Rick Jones @ 2011-10-05 18:08 UTC (permalink / raw)
  To: netdev

From: Rick Jones <rick.jones2@hp.com>

Add support for reporting ring sizes via ethtool -g to the 8139cp driver.

Signed-off-by: Rick Jones <rick.jones2@hp.com>

---

diff --git a/drivers/net/ethernet/realtek/8139cp.c b/drivers/net/ethernet/realtek/8139cp.c
index c77d5af..1dcfe57 100644
--- a/drivers/net/ethernet/realtek/8139cp.c
+++ b/drivers/net/ethernet/realtek/8139cp.c
@@ -1324,6 +1324,19 @@ static void cp_get_drvinfo (struct net_device *dev, struct ethtool_drvinfo *info
 	strcpy (info->bus_info, pci_name(cp->pdev));
 }
 
+static void cp_get_ringparam(struct net_device *dev,
+				struct ethtool_ringparam *ring)
+{
+	ring->rx_max_pending = CP_RX_RING_SIZE;
+	ring->tx_max_pending = CP_TX_RING_SIZE;
+	ring->rx_mini_max_pending = 0;
+	ring->rx_jumbo_max_pending = 0;
+	ring->rx_pending = CP_RX_RING_SIZE;
+	ring->tx_pending = CP_TX_RING_SIZE;
+	ring->rx_mini_pending = 0;
+	ring->rx_jumbo_pending = 0;
+}
+
 static int cp_get_regs_len(struct net_device *dev)
 {
 	return CP_REGS_SIZE;
@@ -1525,6 +1538,7 @@ static const struct ethtool_ops cp_ethtool_ops = {
 	.get_eeprom_len		= cp_get_eeprom_len,
 	.get_eeprom		= cp_get_eeprom,
 	.set_eeprom		= cp_set_eeprom,
+	.get_ringparam		= cp_get_ringparam,
 };
 
 static int cp_ioctl (struct net_device *dev, struct ifreq *rq, int cmd)

^ permalink raw reply related

* RCU list entry in ipv6_chk_addr function
From: Eduardo Panisset @ 2011-10-05 18:16 UTC (permalink / raw)
  To: netdev

Hi All,

I was looking over ipv6 implementation for kernel 3.0.4.

Shouldn't "hlist_for_each_entry_rcu_bh" be used instead of
"hlist_for_each_entry_rcu" in the function ipv6_chk_addr (file
net/ipv6/addrconf.c) ?

Best Regards,
Eduardo Panisset.

^ permalink raw reply

* Re: [PATCH net-next] Add ethtool -g support to 8139cp
From: Ben Hutchings @ 2011-10-05 18:50 UTC (permalink / raw)
  To: Rick Jones; +Cc: netdev
In-Reply-To: <20111005180854.3D68129003A8@tardy>

On Wed, 2011-10-05 at 11:08 -0700, Rick Jones wrote:
> From: Rick Jones <rick.jones2@hp.com>
> 
> Add support for reporting ring sizes via ethtool -g to the 8139cp driver.
> 
> Signed-off-by: Rick Jones <rick.jones2@hp.com>
> 
> ---
> 
> diff --git a/drivers/net/ethernet/realtek/8139cp.c b/drivers/net/ethernet/realtek/8139cp.c
> index c77d5af..1dcfe57 100644
> --- a/drivers/net/ethernet/realtek/8139cp.c
> +++ b/drivers/net/ethernet/realtek/8139cp.c
> @@ -1324,6 +1324,19 @@ static void cp_get_drvinfo (struct net_device *dev, struct ethtool_drvinfo *info
>  	strcpy (info->bus_info, pci_name(cp->pdev));
>  }
>  
> +static void cp_get_ringparam(struct net_device *dev,
> +				struct ethtool_ringparam *ring)
> +{
> +	ring->rx_max_pending = CP_RX_RING_SIZE;
> +	ring->tx_max_pending = CP_TX_RING_SIZE;
> +	ring->rx_mini_max_pending = 0;
> +	ring->rx_jumbo_max_pending = 0;
> +	ring->rx_pending = CP_RX_RING_SIZE;
> +	ring->tx_pending = CP_TX_RING_SIZE;
> +	ring->rx_mini_pending = 0;
> +	ring->rx_jumbo_pending = 0;
> +}

You generally don't need to set fields to zero in ethtool 'get'
operations, as the ethtool core will initialise the entire structure to
zero.

Ben.

>  static int cp_get_regs_len(struct net_device *dev)
>  {
>  	return CP_REGS_SIZE;
> @@ -1525,6 +1538,7 @@ static const struct ethtool_ops cp_ethtool_ops = {
>  	.get_eeprom_len		= cp_get_eeprom_len,
>  	.get_eeprom		= cp_get_eeprom,
>  	.set_eeprom		= cp_set_eeprom,
> +	.get_ringparam		= cp_get_ringparam,
>  };
>  
>  static int cp_ioctl (struct net_device *dev, struct ifreq *rq, int cmd)
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Ben Hutchings, Staff Engineer, Solarflare
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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox