Netdev List
 help / color / mirror / Atom feed
* [net 1/2] e1000e: MSI interrupt test failed, using legacy interrupt
From: Jeff Kirsher @ 2012-04-25  5:55 UTC (permalink / raw)
  To: davem; +Cc: Prasanna S Panchamukhi, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1335333314-6814-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Prasanna S Panchamukhi <ppanchamukhi@riverbed.com>

Following logs where seen on Systems with multiple NICs,
while using MSI interrupts as shown below:

Feb 16 15:09:32 (none) user.notice kernel: 0000:00:0d.0: lan0_0: NIC Link is Up
1000 Mbps Full Duplex, Flow Control: RX/TX
Feb 16 15:09:32 (none) user.notice kernel: 0000:40:0d.0: wan0_1: NIC Link is Up
1000 Mbps Full Duplex, Flow Control: RX/TX
Feb 16 15:09:32 (none) user.notice kernel: 0000:40:0d.0: lan0_1: NIC Link is Up
1000 Mbps Full Duplex, Flow Control: RX/TX
Feb 16 15:09:32 (none) user.warn kernel: 0000:40:0e.0: wan4_0: MSI interrupt
test failed, using legacy interrupt.
Feb 16 15:09:32 (none) user.notice kernel: 0000:00:0e.0: wan1_0: NIC Link is Up
1000 Mbps Full Duplex, Flow Control: RX/TX
Feb 16 15:09:33 (none) user.notice kernel: 0000:00:0e.0: lan1_0: NIC Link is Up
1000 Mbps Full Duplex, Flow Control: RX/TX
Feb 16 15:09:33 (none) user.notice kernel: 0000:00:0f.0: wan2_0: NIC Link is Up
1000 Mbps Full Duplex, Flow Control: RX/TX
Feb 16 15:09:33 (none) user.notice kernel: 0000:00:0f.0: lan2_0: NIC Link is Up
1000 Mbps Full Duplex, Flow Control: RX/TX
Feb 16 15:09:33 (none) user.notice kernel: 0000:40:0a.0: wan3_0: NIC Link is Up
1000 Mbps Full Duplex, Flow Control: RX/TX
Feb 16 15:09:33 (none) user.notice kernel: 0000:40:0a.0: lan3_0: NIC Link is Up
1000 Mbps Full Duplex, Flow Control: RX/TX
Feb 16 15:09:34 (none) user.notice kernel: 0000:40:0e.0: lan4_0: NIC Link is Up
1000 Mbps Full Duplex, Flow Control: RX/TX
Feb 16 15:09:34 (none) user.notice kernel: 0000:40:0f.0: wan5_0: NIC Link is Up
1000 Mbps Full Duplex, Flow Control: RX/TX
Feb 16 15:09:34 (none) user.notice kernel: 0000:40:0f.0: lan5_0: NIC Link is Up
1000 Mbps Full Duplex, Flow Control: RX/TX

This patch fixes this problem by increasing the msleep from 50 to 100.

Signed-off-by: Prasanna S Panchamukhi <ppanchamukhi@riverbed.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/e1000e/netdev.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 19ab215..9520a6a 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -3799,7 +3799,7 @@ static int e1000_test_msi_interrupt(struct e1000_adapter *adapter)
 	/* fire an unusual interrupt on the test handler */
 	ew32(ICS, E1000_ICS_RXSEQ);
 	e1e_flush();
-	msleep(50);
+	msleep(100);
 
 	e1000_irq_disable(adapter);
 
-- 
1.7.7.6

^ permalink raw reply related

* [net 0/2][pull request] Intel Wired LAN Driver Updates
From: Jeff Kirsher @ 2012-04-25  5:55 UTC (permalink / raw)
  To: davem; +Cc: Jeff Kirsher, netdev, gospo, sassmann

This series of patches contains fixes for e1000e only.

The following are changes since commit 2a5809499e35b53a6044fd34e72b242688b7a862:
  asix: Fix tx transfer padding for full-speed USB
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net master

Jeff Kirsher (1):
  e1000e: Fix default interrupt throttle rate not set in NIC HW

Prasanna S Panchamukhi (1):
  e1000e: MSI interrupt test failed, using legacy interrupt

 drivers/net/ethernet/intel/e1000e/netdev.c |    2 +-
 drivers/net/ethernet/intel/e1000e/param.c  |   99 +++++++++++++++-------------
 2 files changed, 54 insertions(+), 47 deletions(-)

-- 
1.7.7.6

^ permalink raw reply

* Re: [PATCH net-next] ipv6: RTAX_FEATURE_ALLFRAG causes inefficient TCP segment sizing
From: Eric Dumazet @ 2012-04-25  5:32 UTC (permalink / raw)
  To: Maciej Żenczykowski; +Cc: Tore Anderson, David Miller, netdev, Tom Herbert
In-Reply-To: <CANP3RGfhc+4FjB152r58AP66oZQmj0oY-Zh=9qNn9OABCXPwbw@mail.gmail.com>

On Tue, 2012-04-24 at 14:50 -0700, Maciej Żenczykowski wrote:
> On Tue, Apr 24, 2012 at 1:10 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > On Tue, 2012-04-24 at 12:49 -0700, Maciej Żenczykowski wrote:
> >> Why do we refuse to set ipv6 mtu's below 1280?
> >> how is what we do any better?
> >
> > I guess you didnt read Tore use case.
> >
> > Thats the standard : http://tools.ietf.org/html/rfc2460#section-5
> >
> >   In response to an IPv6 packet that is sent to an IPv4 destination
> >   (i.e., a packet that undergoes translation from IPv6 to IPv4), the
> >   originating IPv6 node may receive an ICMP Packet Too Big message
> >   reporting a Next-Hop MTU less than 1280.  In that case, the IPv6 node
> >   is not required to reduce the size of subsequent packets to less than
> 
> ... is not required to reduce...
> 
> but doesn't that mean it _may_ reduce anyway?
> and thus make life easier on the guy who will have to fragment (and
> thus he won't have to fragment).
> 
> [of course you'd still need to lose 8 bytes for the frag header...]
> 
> >   1280, but must include a Fragment header in those packets so that the
> >   IPv6-to-IPv4 translating router can obtain a suitable Identification
> >   value to use in resulting IPv4 fragments.  Note that this means the
> >   payload may have to be reduced to 1232 octets (1280 minus 40 for the
> >   IPv6 header and 8 for the Fragment header), and smaller still if
> >   additional extension headers are used.
> 
> I just don't see what not decreasing mtu below 1280 buys us.

But we chose to _not_ decrease mtu and adhere to the specs.

Current linux chose to implement the allfragfeature, so match RFC 2460
specs.

If you want to reduce size of subsequent packets, its a lot more work in
linux stack, for small gain, if any.

We only need to properly generate the fragment header, that means
reducing the _payload_ by 8 bytes. Not reducing the _mtu_ that still is
1280, as allowed.

IPv6 must cohabit with IPv4 for the next years, there is no hope
thinking it can ignore tunnelings issues, since tunneling is part of the
global IPv6 transition that is currently happening.

^ permalink raw reply

* [PATCH net-next] net: sock_diag_handler structs can be const
From: Shan Wei @ 2012-04-25  4:21 UTC (permalink / raw)
  To: David Miller, xemul; +Cc: NetDev, davidshan

From: Shan Wei <davidshan@tencent.com>

read only, so change it to const.

Signed-off-by: Shan Wei <davidshan@tencent.com>
---
 include/linux/sock_diag.h |    4 ++--
 net/core/sock_diag.c      |   12 ++++++------
 net/ipv4/inet_diag.c      |    4 ++--
 net/unix/diag.c           |    2 +-
 4 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/include/linux/sock_diag.h b/include/linux/sock_diag.h
index 251729a..db4bae7 100644
--- a/include/linux/sock_diag.h
+++ b/include/linux/sock_diag.h
@@ -32,8 +32,8 @@ struct sock_diag_handler {
 	int (*dump)(struct sk_buff *skb, struct nlmsghdr *nlh);
 };
 
-int sock_diag_register(struct sock_diag_handler *h);
-void sock_diag_unregister(struct sock_diag_handler *h);
+int sock_diag_register(const struct sock_diag_handler *h);
+void sock_diag_unregister(const struct sock_diag_handler *h);
 
 void sock_diag_register_inet_compat(int (*fn)(struct sk_buff *skb, struct nlmsghdr *nlh));
 void sock_diag_unregister_inet_compat(int (*fn)(struct sk_buff *skb, struct nlmsghdr *nlh));
diff --git a/net/core/sock_diag.c b/net/core/sock_diag.c
index b9868e1..5fd1467 100644
--- a/net/core/sock_diag.c
+++ b/net/core/sock_diag.c
@@ -10,7 +10,7 @@
 #include <linux/inet_diag.h>
 #include <linux/sock_diag.h>
 
-static struct sock_diag_handler *sock_diag_handlers[AF_MAX];
+static const struct sock_diag_handler *sock_diag_handlers[AF_MAX];
 static int (*inet_rcv_compat)(struct sk_buff *skb, struct nlmsghdr *nlh);
 static DEFINE_MUTEX(sock_diag_table_mutex);
 
@@ -70,7 +70,7 @@ void sock_diag_unregister_inet_compat(int (*fn)(struct sk_buff *skb, struct nlms
 }
 EXPORT_SYMBOL_GPL(sock_diag_unregister_inet_compat);
 
-int sock_diag_register(struct sock_diag_handler *hndl)
+int sock_diag_register(const struct sock_diag_handler *hndl)
 {
 	int err = 0;
 
@@ -88,7 +88,7 @@ int sock_diag_register(struct sock_diag_handler *hndl)
 }
 EXPORT_SYMBOL_GPL(sock_diag_register);
 
-void sock_diag_unregister(struct sock_diag_handler *hnld)
+void sock_diag_unregister(const struct sock_diag_handler *hnld)
 {
 	int family = hnld->family;
 
@@ -102,7 +102,7 @@ void sock_diag_unregister(struct sock_diag_handler *hnld)
 }
 EXPORT_SYMBOL_GPL(sock_diag_unregister);
 
-static inline struct sock_diag_handler *sock_diag_lock_handler(int family)
+static const inline struct sock_diag_handler *sock_diag_lock_handler(int family)
 {
 	if (sock_diag_handlers[family] == NULL)
 		request_module("net-pf-%d-proto-%d-type-%d", PF_NETLINK,
@@ -112,7 +112,7 @@ static inline struct sock_diag_handler *sock_diag_lock_handler(int family)
 	return sock_diag_handlers[family];
 }
 
-static inline void sock_diag_unlock_handler(struct sock_diag_handler *h)
+static inline void sock_diag_unlock_handler(const struct sock_diag_handler *h)
 {
 	mutex_unlock(&sock_diag_table_mutex);
 }
@@ -121,7 +121,7 @@ static int __sock_diag_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 {
 	int err;
 	struct sock_diag_req *req = NLMSG_DATA(nlh);
-	struct sock_diag_handler *hndl;
+	const struct sock_diag_handler *hndl;
 
 	if (nlmsg_len(nlh) < sizeof(*req))
 		return -EINVAL;
diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
index 8f8db72..46d1e71 100644
--- a/net/ipv4/inet_diag.c
+++ b/net/ipv4/inet_diag.c
@@ -999,12 +999,12 @@ static int inet_diag_handler_dump(struct sk_buff *skb, struct nlmsghdr *h)
 	return inet_diag_get_exact(skb, h, (struct inet_diag_req_v2 *)NLMSG_DATA(h));
 }
 
-static struct sock_diag_handler inet_diag_handler = {
+static const struct sock_diag_handler inet_diag_handler = {
 	.family = AF_INET,
 	.dump = inet_diag_handler_dump,
 };
 
-static struct sock_diag_handler inet6_diag_handler = {
+static const struct sock_diag_handler inet6_diag_handler = {
 	.family = AF_INET6,
 	.dump = inet_diag_handler_dump,
 };
diff --git a/net/unix/diag.c b/net/unix/diag.c
index f0486ae..47d3002 100644
--- a/net/unix/diag.c
+++ b/net/unix/diag.c
@@ -310,7 +310,7 @@ static int unix_diag_handler_dump(struct sk_buff *skb, struct nlmsghdr *h)
 		return unix_diag_get_exact(skb, h, (struct unix_diag_req *)NLMSG_DATA(h));
 }
 
-static struct sock_diag_handler unix_diag_handler = {
+static const struct sock_diag_handler unix_diag_handler = {
 	.family = AF_UNIX,
 	.dump = unix_diag_handler_dump,
 };
-- 
1.7.1

^ permalink raw reply related

* [PATCH BUG-FIX] udp_diag: implement idiag_get_info for udp/udplite to get queue information
From: Shan Wei @ 2012-04-25  4:15 UTC (permalink / raw)
  To: David Miller, kuznet, jmorris, xemul; +Cc: NetDev, davidshan

From: Shan Wei <davidshan@tencent.com>

When we use netlink to monitor queue information for udp socket,
idiag_rqueue and idiag_wqueue of inet_diag_msg are returned with 0.

Keep consistent with netstat, just return back allocated rmem/wmem size.

Signed-off-by: Shan Wei <davidshan@tencent.com>
---
 net/ipv4/inet_diag.c |    2 +-
 net/ipv4/udp_diag.c  |    9 +++++++++
 2 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
index 8d25a1c..8f8db72 100644
--- a/net/ipv4/inet_diag.c
+++ b/net/ipv4/inet_diag.c
@@ -141,7 +141,7 @@ int inet_sk_diag_fill(struct sock *sk, struct inet_connection_sock *icsk,
 			goto rtattr_failure;
 
 	if (icsk == NULL) {
-		r->idiag_rqueue = r->idiag_wqueue = 0;
+		handler->idiag_get_info(sk, r, NULL);
 		goto out;
 	}
 
diff --git a/net/ipv4/udp_diag.c b/net/ipv4/udp_diag.c
index 8a949f1..a7f86a3 100644
--- a/net/ipv4/udp_diag.c
+++ b/net/ipv4/udp_diag.c
@@ -146,9 +146,17 @@ static int udp_diag_dump_one(struct sk_buff *in_skb, const struct nlmsghdr *nlh,
 	return udp_dump_one(&udp_table, in_skb, nlh, req);
 }
 
+static void udp_diag_get_info(struct sock *sk, struct inet_diag_msg *r,
+		void *info)
+{
+	r->idiag_rqueue = sk_rmem_alloc_get(sk);
+	r->idiag_wqueue = sk_wmem_alloc_get(sk);
+}
+
 static const struct inet_diag_handler udp_diag_handler = {
 	.dump		 = udp_diag_dump,
 	.dump_one	 = udp_diag_dump_one,
+	.idiag_get_info  = udp_diag_get_info,
 	.idiag_type	 = IPPROTO_UDP,
 };
 
@@ -167,6 +175,7 @@ static int udplite_diag_dump_one(struct sk_buff *in_skb, const struct nlmsghdr *
 static const struct inet_diag_handler udplite_diag_handler = {
 	.dump		 = udplite_diag_dump,
 	.dump_one	 = udplite_diag_dump_one,
+	.idiag_get_info  = udp_diag_get_info,
 	.idiag_type	 = IPPROTO_UDPLITE,
 };
 
-- 
1.7.1

^ permalink raw reply related

* Question on IPv6 addresses and netlink message ordering
From: Pradeep Kanyar @ 2012-04-25  3:11 UTC (permalink / raw)
  To: netdev
In-Reply-To: <CAL7k4TzFHavRETDo-myeQonR0RyA35v5uwe1NKaxREn3E1xdZg@mail.gmail.com>

Hi,
If we configure a global IPv6 address on an interface in linux, and if
we monitor the netlink messages
[using nl-monitor from libnl for e.g,] I see that we get RTM_NEWROUTE
for the global address subnet
first, and then RTM_NEWADDR. Isn't this strange? Shouldn't we be
seeing RTM_NEWADDR first
andRTM_NEWROUTE subsequently ....? I put some prints in the path and I
see that the following
seems to be order of events [initially at least] when I configure a
global v6 address on an interface:

1. RTM_NEWROUTE notify in fib6_add_rt2node
2. RTM_NEWADDR notify in addrconf_dad_completed

I saw this on a system running 2.6.32 but a cursory look at more
recent kernels seemed to indicate
that the code path hasn't changed much in this area [ but I might be wrong ].

Why is this out-of-order? On the contrary, in case of IPv4 addresses,
we always see RTM_NEWADDR
first followed by RTM_NEWROUTE [for the directly connected network]

Thanks,
Pradeep

^ permalink raw reply

* [PATCH] crush: include header for global symbols
From: H Hartley Sweeten @ 2012-04-25  0:38 UTC (permalink / raw)
  To: Linux Kernel; +Cc: netdev, ceph-devel, sage, davem

Include the header to pickup the definitions of the global symbols.

Quiets the following sparse warnings:

warning: symbol 'crush_find_rule' was not declared. Should it be static?
warning: symbol 'crush_do_rule' was not declared. Should it be static?

Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
Cc: Sage Weil <sage@newdream.net>
Cc: "David S. Miller" <davem@davemloft.net>

---

diff --git a/net/ceph/crush/mapper.c b/net/ceph/crush/mapper.c
index 854ac53..363f8f7 100644
--- a/net/ceph/crush/mapper.c
+++ b/net/ceph/crush/mapper.c
@@ -20,6 +20,7 @@
 
 #include <linux/crush/crush.h>
 #include <linux/crush/hash.h>
+#include <linux/crush/mapper.h>
 
 /*
  * Implement the core CRUSH mapping algorithm.

^ permalink raw reply related

* Re: [PATCH v2 5/5] decrement static keys on real destroy time
From: KAMEZAWA Hiroyuki @ 2012-04-25  0:22 UTC (permalink / raw)
  To: Glauber Costa; +Cc: Tejun Heo, netdev, cgroups, Li Zefan, David Miller, devel
In-Reply-To: <4F969176.8010804@parallels.com>

(2012/04/24 20:41), Glauber Costa wrote:

> On 04/23/2012 11:40 PM, KAMEZAWA Hiroyuki wrote:
>> (2012/04/24 4:37), Glauber Costa wrote:
>>
>>> We call the destroy function when a cgroup starts to be removed,
>>> such as by a rmdir event.
>>>
>>> However, because of our reference counters, some objects are still
>>> inflight. Right now, we are decrementing the static_keys at destroy()
>>> time, meaning that if we get rid of the last static_key reference,
>>> some objects will still have charges, but the code to properly
>>> uncharge them won't be run.
>>>
>>> This becomes a problem specially if it is ever enabled again, because
>>> now new charges will be added to the staled charges making keeping
>>> it pretty much impossible.
>>>
>>> We just need to be careful with the static branch activation:
>>> since there is no particular preferred order of their activation,
>>> we need to make sure that we only start using it after all
>>> call sites are active. This is achieved by having a per-memcg
>>> flag that is only updated after static_key_slow_inc() returns.
>>> At this time, we are sure all sites are active.
>>>
>>> This is made per-memcg, not global, for a reason:
>>> it also has the effect of making socket accounting more
>>> consistent. The first memcg to be limited will trigger static_key()
>>> activation, therefore, accounting. But all the others will then be
>>> accounted no matter what. After this patch, only limited memcgs
>>> will have its sockets accounted.
>>>
>>> [v2: changed a tcp limited flag for a generic proto limited flag ]
>>> [v3: update the current active flag only after the static_key update ]
>>>
>>> Signed-off-by: Glauber Costa<glommer@parallels.com>
>>
>>
>> Acked-by: KAMEZAWA Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com>
>>
>> A small request below.
>>
>> <snip>
>>
>>
>>> +		 * ->activated needs to be written after the static_key update.
>>> +		 *  This is what guarantees that the socket activation function
>>> +		 *  is the last one to run. See sock_update_memcg() for details,
>>> +		 *  and note that we don't mark any socket as belonging to this
>>> +		 *  memcg until that flag is up.
>>> +		 *
>>> +		 *  We need to do this, because static_keys will span multiple
>>> +		 *  sites, but we can't control their order. If we mark a socket
>>> +		 *  as accounted, but the accounting functions are not patched in
>>> +		 *  yet, we'll lose accounting.
>>> +		 *
>>> +		 *  We never race with the readers in sock_update_memcg(), because
>>> +		 *  when this value change, the code to process it is not patched in
>>> +		 *  yet.
>>> +		 */
>>> +		mutex_lock(&tcp_set_limit_mutex);
>>
>>
>> Could you explain for what this mutex is in above comment ?
>>
> This is explained at the site where the mutex is defined.
> If you still want me to mention it here, or maybe expand the explanation
> there, I surely can.
> 

Ah, I think it's better to mention one more complicated race.
Let me explain. 

Assume we don't have tcp_set_limit_mutex. And jump_label is not activated yet
i.e. memcg_socket_limit_enabled->count == 0.

When a user updates limit of 2 cgroups at once, following happens.

	CPU A				CPU B

    if (cg_proto->activated)	   if (cg->proto_activated)
        static_key_inc()                static_key_inc()
        => set counter 0->1             => set counter 1->2, return immediately.
           => hold mutex                => cg_proto->activated = true.  
              => overwrite jmps.

Then, without mutex, activated/active may be set 'true' before the end
of jump_label modification.

Thanks,
-Kame

^ permalink raw reply

* Re: Netlink for kernel<->user space communication?
From: Stephen Hemminger @ 2012-04-24 23:57 UTC (permalink / raw)
  To: Arvid Brodin; +Cc: netdev@vger.kernel.org
In-Reply-To: <4F973CC1.8000002@xdin.com>

On Tue, 24 Apr 2012 23:52:34 +0000
Arvid Brodin <Arvid.Brodin@xdin.com> wrote:

> Hi.
> 
> I'm writing a kernel driver for the HSR protocol, a standard for high availability
> networks. I want to send messages from the kernel to user space about broken network
> links. I also want user space to be able to ask the kernel about its view of the status of
> nodes on the network.
> 
> Netlink seems like a good tool for this. (Is it?)

Yes.

> But do I use raw netlink? (Described here: http://www.linuxjournal.com/article/7356 - but
> this seems a bit out of date, the kernel API description differs from today's kernel
> implementation.)

No. Your driver probably looks like a device so you should be
using rtnetlink messages.

> Or do I use the "Kernel Connector" (Documentation/connector/connector.txt)?
no.

> Do I use libnetlink?
if you extend iproute2 to support your link type, then yes,
but use the version inside iproute2 (rather than the older extracted libnetlink
in some distros).

> Or do I use libnl? (This seems to be actively maintained.)
If you aren't going to be in iproute2 then use libmnl. libnl does lots
of caching etc, which makes it good for monitoring and gui tools but a pain
for simple management.

Advice is free, code is what counts.

^ permalink raw reply

* Netlink for kernel<->user space communication?
From: Arvid Brodin @ 2012-04-24 23:52 UTC (permalink / raw)
  To: netdev@vger.kernel.org

Hi.

I'm writing a kernel driver for the HSR protocol, a standard for high availability
networks. I want to send messages from the kernel to user space about broken network
links. I also want user space to be able to ask the kernel about its view of the status of
nodes on the network.

Netlink seems like a good tool for this. (Is it?)

But do I use raw netlink? (Described here: http://www.linuxjournal.com/article/7356 - but
this seems a bit out of date, the kernel API description differs from today's kernel
implementation.)
Or do I use the "Kernel Connector" (Documentation/connector/connector.txt)?
Do I use libnetlink?
Or do I use libnl? (This seems to be actively maintained.)
Or are there more and even better ways to do this?

I'm having a case of information overload here and would be grateful for some guidelines.

Thanks,
Arvid Brodin
Enea Services Stockholm AB - since February 16 a part of Xdin in the Alten Group. Soon we
will be working under the common brand name Xdin. Read more at www.xdin.com.

^ permalink raw reply

* [PATCH net] tg3: Avoid panic from reserved statblk field access
From: Matt Carlson @ 2012-04-24 23:37 UTC (permalink / raw)
  To: davem; +Cc: netdev, mcarlson, mchan

When RSS is enabled, interrupt vector 0 does not receive any rx traffic.
The rx producer index fields for vector 0's status block should be
considered reserved in this case.  This patch changes the code to
respect these reserved fields, which avoids a kernel panic when these
fields take on non-zero values.

Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
Signed-off-by: Michael Chan <mchan@broadcom.com>
---
 drivers/net/ethernet/broadcom/tg3.c |   18 ++++++++++++++++--
 1 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 062ac33..ceeab8e 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -879,8 +879,13 @@ static inline unsigned int tg3_has_work(struct tg3_napi *tnapi)
 		if (sblk->status & SD_STATUS_LINK_CHG)
 			work_exists = 1;
 	}
-	/* check for RX/TX work to do */
-	if (sblk->idx[0].tx_consumer != tnapi->tx_cons ||
+
+	/* check for TX work to do */
+	if (sblk->idx[0].tx_consumer != tnapi->tx_cons)
+		work_exists = 1;
+
+	/* check for RX work to do */
+	if (tnapi->rx_rcb_prod_idx &&
 	    *(tnapi->rx_rcb_prod_idx) != tnapi->rx_rcb_ptr)
 		work_exists = 1;
 
@@ -6124,6 +6129,9 @@ static int tg3_poll_work(struct tg3_napi *tnapi, int work_done, int budget)
 			return work_done;
 	}
 
+	if (!tnapi->rx_rcb_prod_idx)
+		return work_done;
+
 	/* run RX thread, within the bounds set by NAPI.
 	 * All RX "locking" is done by ensuring outside
 	 * code synchronizes with tg3->napi.poll()
@@ -7567,6 +7575,12 @@ static int tg3_alloc_consistent(struct tg3 *tp)
 		 */
 		switch (i) {
 		default:
+			if (tg3_flag(tp, ENABLE_RSS)) {
+				tnapi->rx_rcb_prod_idx = NULL;
+				break;
+			}
+			/* Fall through */
+		case 1:
 			tnapi->rx_rcb_prod_idx = &sblk->idx[0].rx_producer;
 			break;
 		case 2:
-- 
1.7.3.4

^ permalink raw reply related

* [PATCH] mISDN: Ignore return value of driver_for_each_device in function hfcpci_softirq.
From: Krzysztof Wilczynski @ 2012-04-24 23:13 UTC (permalink / raw)
  To: Karsten Keil
  Cc: Greg Kroah-Hartman, Simon Horman, Jiri Kosina, netdev,
	linux-kernel

Function hfcpci_softirq uses driver_for_each_device to call _hfcpci_softirq for
each device. Given that _hfcpci_softirq always returns 0, we collect this return
value and simply do nothing with it.  This is purely to address the following
warning during compilation time:

  drivers/isdn/hardware/mISDN/hfcpci.c: In function ‘hfcpci_softirq’:
  drivers/isdn/hardware/mISDN/hfcpci.c:2319: warning: ignoring return value of
    ‘driver_for_each_device’, declared with attribute warn_unused_result

Similar approach was used in function cx18_alsa_exit from "cx18-alsa-main.c".

Signed-off-by: Krzysztof Wilczynski <krzysztof.wilczynski@linux.com>
---
 drivers/isdn/hardware/mISDN/hfcpci.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/isdn/hardware/mISDN/hfcpci.c b/drivers/isdn/hardware/mISDN/hfcpci.c
index e2c83a2..f0d0180 100644
--- a/drivers/isdn/hardware/mISDN/hfcpci.c
+++ b/drivers/isdn/hardware/mISDN/hfcpci.c
@@ -2316,8 +2316,11 @@ _hfcpci_softirq(struct device *dev, void *arg)
 static void
 hfcpci_softirq(void *arg)
 {
-	(void) driver_for_each_device(&hfc_driver.driver, NULL, arg,
-				      _hfcpci_softirq);
+	int ret;
+
+	/* Collect return value, but do not use it; suppress GCC warning ... */
+	ret = driver_for_each_device(&hfc_driver.driver, NULL, arg,
+				     _hfcpci_softirq);
 
 	/* if next event would be in the past ... */
 	if ((s32)(hfc_jiffies + tics - jiffies) <= 0)
-- 
1.7.2.5

^ permalink raw reply related

* [PATCH] isdn/eicon: use standard __init,__exit function markup
From: H Hartley Sweeten @ 2012-04-24 22:56 UTC (permalink / raw)
  To: Linux Kernel; +Cc: netdev, mac, isdn

Remove the custom DIVA_{INIT,EXIT}_FUNCTION defines and use
the standard __init,__exit markup.

Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
Cc: Armin Schindler <mac@melware.de>
Cc: Karsten Keil <isdn@linux-pingi.de>

---

diff --git a/drivers/isdn/hardware/eicon/capifunc.c b/drivers/isdn/hardware/eicon/capifunc.c
index a576f32..7a0bdbd 100644
--- a/drivers/isdn/hardware/eicon/capifunc.c
+++ b/drivers/isdn/hardware/eicon/capifunc.c
@@ -1120,7 +1120,7 @@ int fax_head_line_time(char *buffer)
 /*
  * init (alloc) main structures
  */
-static int DIVA_INIT_FUNCTION init_main_structs(void)
+static int __init init_main_structs(void)
 {
 	if (!(mapped_msg = (CAPI_MSG *) diva_os_malloc(0, MAX_MSG_SIZE))) {
 		DBG_ERR(("init: failed alloc mapped_msg."))
@@ -1181,7 +1181,7 @@ static void do_api_remove_start(void)
 /*
  * init
  */
-int DIVA_INIT_FUNCTION init_capifunc(void)
+int __init init_capifunc(void)
 {
 	diva_os_initialize_spin_lock(&api_lock, "capifunc");
 	memset(ControllerMap, 0, MAX_DESCRIPTORS + 1);
@@ -1209,7 +1209,7 @@ int DIVA_INIT_FUNCTION init_capifunc(void)
 /*
  * finit
  */
-void DIVA_EXIT_FUNCTION finit_capifunc(void)
+void __exit finit_capifunc(void)
 {
 	do_api_remove_start();
 	divacapi_disconnect_didd();
diff --git a/drivers/isdn/hardware/eicon/capimain.c b/drivers/isdn/hardware/eicon/capimain.c
index eabe0fa..997d46a 100644
--- a/drivers/isdn/hardware/eicon/capimain.c
+++ b/drivers/isdn/hardware/eicon/capimain.c
@@ -118,7 +118,7 @@ void diva_os_set_controller_struct(struct capi_ctr *ctrl)
 /*
  * module init
  */
-static int DIVA_INIT_FUNCTION divacapi_init(void)
+static int __init divacapi_init(void)
 {
 	char tmprev[32];
 	int ret = 0;
@@ -144,7 +144,7 @@ static int DIVA_INIT_FUNCTION divacapi_init(void)
 /*
  * module exit
  */
-static void DIVA_EXIT_FUNCTION divacapi_exit(void)
+static void __exit divacapi_exit(void)
 {
 	finit_capifunc();
 	printk(KERN_INFO "%s: module unloaded.\n", DRIVERLNAME);
diff --git a/drivers/isdn/hardware/eicon/diddfunc.c b/drivers/isdn/hardware/eicon/diddfunc.c
index c4c8220..b0b23ed 100644
--- a/drivers/isdn/hardware/eicon/diddfunc.c
+++ b/drivers/isdn/hardware/eicon/diddfunc.c
@@ -47,7 +47,7 @@ static void *didd_callback(void *context, DESCRIPTOR *adapter,
 /*
  * connect to didd
  */
-static int DIVA_INIT_FUNCTION connect_didd(void)
+static int __init connect_didd(void)
 {
 	int x = 0;
 	int dadapter = 0;
@@ -79,7 +79,7 @@ static int DIVA_INIT_FUNCTION connect_didd(void)
 /*
  * disconnect from didd
  */
-static void DIVA_EXIT_FUNCTION disconnect_didd(void)
+static void __exit disconnect_didd(void)
 {
 	IDI_SYNC_REQ req;
 
@@ -92,7 +92,7 @@ static void DIVA_EXIT_FUNCTION disconnect_didd(void)
 /*
  * init
  */
-int DIVA_INIT_FUNCTION diddfunc_init(void)
+int __init diddfunc_init(void)
 {
 	diva_didd_load_time_init();
 
@@ -107,7 +107,7 @@ int DIVA_INIT_FUNCTION diddfunc_init(void)
 /*
  * finit
  */
-void DIVA_EXIT_FUNCTION diddfunc_finit(void)
+void __exit diddfunc_finit(void)
 {
 	DbgDeregister();
 	disconnect_didd();
diff --git a/drivers/isdn/hardware/eicon/diva_didd.c b/drivers/isdn/hardware/eicon/diva_didd.c
index d1d3de0..fab6ccf 100644
--- a/drivers/isdn/hardware/eicon/diva_didd.c
+++ b/drivers/isdn/hardware/eicon/diva_didd.c
@@ -91,7 +91,7 @@ static const struct file_operations divadidd_proc_fops = {
 	.release	= single_release,
 };
 
-static int DIVA_INIT_FUNCTION create_proc(void)
+static int __init create_proc(void)
 {
 	proc_net_eicon = proc_mkdir("eicon", init_net.proc_net);
 
@@ -109,7 +109,7 @@ static void remove_proc(void)
 	remove_proc_entry("eicon", init_net.proc_net);
 }
 
-static int DIVA_INIT_FUNCTION divadidd_init(void)
+static int __init divadidd_init(void)
 {
 	char tmprev[32];
 	int ret = 0;
@@ -141,7 +141,7 @@ out:
 	return (ret);
 }
 
-static void DIVA_EXIT_FUNCTION divadidd_exit(void)
+static void __exit divadidd_exit(void)
 {
 	diddfunc_finit();
 	remove_proc();
diff --git a/drivers/isdn/hardware/eicon/divamnt.c b/drivers/isdn/hardware/eicon/divamnt.c
index ffa0c31..48db08d 100644
--- a/drivers/isdn/hardware/eicon/divamnt.c
+++ b/drivers/isdn/hardware/eicon/divamnt.c
@@ -184,7 +184,7 @@ static void divas_maint_unregister_chrdev(void)
 	unregister_chrdev(major, DEVNAME);
 }
 
-static int DIVA_INIT_FUNCTION divas_maint_register_chrdev(void)
+static int __init divas_maint_register_chrdev(void)
 {
 	if ((major = register_chrdev(0, DEVNAME, &divas_maint_fops)) < 0)
 	{
@@ -207,7 +207,7 @@ void diva_maint_wakeup_read(void)
 /*
  *  Driver Load
  */
-static int DIVA_INIT_FUNCTION maint_init(void)
+static int __init maint_init(void)
 {
 	char tmprev[50];
 	int ret = 0;
@@ -245,7 +245,7 @@ out:
 /*
 **  Driver Unload
 */
-static void DIVA_EXIT_FUNCTION maint_exit(void)
+static void __exit maint_exit(void)
 {
 	divas_maint_unregister_chrdev();
 	mntfunc_finit();
diff --git a/drivers/isdn/hardware/eicon/divasfunc.c b/drivers/isdn/hardware/eicon/divasfunc.c
index 60aaf95..4be5f88 100644
--- a/drivers/isdn/hardware/eicon/divasfunc.c
+++ b/drivers/isdn/hardware/eicon/divasfunc.c
@@ -153,7 +153,7 @@ static void *didd_callback(void *context, DESCRIPTOR *adapter,
 /*
  * connect to didd
  */
-static int DIVA_INIT_FUNCTION connect_didd(void)
+static int __init connect_didd(void)
 {
 	int x = 0;
 	int dadapter = 0;
@@ -209,7 +209,7 @@ static void disconnect_didd(void)
 /*
  * init
  */
-int DIVA_INIT_FUNCTION divasfunc_init(int dbgmask)
+int __init divasfunc_init(int dbgmask)
 {
 	char *version;
 
diff --git a/drivers/isdn/hardware/eicon/divasi.c b/drivers/isdn/hardware/eicon/divasi.c
index a5c8f90..4103a8c 100644
--- a/drivers/isdn/hardware/eicon/divasi.c
+++ b/drivers/isdn/hardware/eicon/divasi.c
@@ -114,7 +114,7 @@ static const struct file_operations um_idi_proc_fops = {
 	.release	= single_release,
 };
 
-static int DIVA_INIT_FUNCTION create_um_idi_proc(void)
+static int __init create_um_idi_proc(void)
 {
 	um_idi_proc_entry = proc_create(DRIVERLNAME, S_IRUGO, proc_net_eicon,
 					&um_idi_proc_fops);
@@ -146,7 +146,7 @@ static void divas_idi_unregister_chrdev(void)
 	unregister_chrdev(major, DEVNAME);
 }
 
-static int DIVA_INIT_FUNCTION divas_idi_register_chrdev(void)
+static int __init divas_idi_register_chrdev(void)
 {
 	if ((major = register_chrdev(0, DEVNAME, &divas_idi_fops)) < 0)
 	{
@@ -161,7 +161,7 @@ static int DIVA_INIT_FUNCTION divas_idi_register_chrdev(void)
 /*
 ** Driver Load
 */
-static int DIVA_INIT_FUNCTION divasi_init(void)
+static int __init divasi_init(void)
 {
 	char tmprev[50];
 	int ret = 0;
@@ -202,7 +202,7 @@ out:
 /*
 ** Driver Unload
 */
-static void DIVA_EXIT_FUNCTION divasi_exit(void)
+static void __exit divasi_exit(void)
 {
 	idifunc_finit();
 	remove_um_idi_proc();
diff --git a/drivers/isdn/hardware/eicon/divasmain.c b/drivers/isdn/hardware/eicon/divasmain.c
index 7eaab06..ca6d276 100644
--- a/drivers/isdn/hardware/eicon/divasmain.c
+++ b/drivers/isdn/hardware/eicon/divasmain.c
@@ -673,7 +673,7 @@ static void divas_unregister_chrdev(void)
 	unregister_chrdev(major, DEVNAME);
 }
 
-static int DIVA_INIT_FUNCTION divas_register_chrdev(void)
+static int __init divas_register_chrdev(void)
 {
 	if ((major = register_chrdev(0, DEVNAME, &divas_fops)) < 0)
 	{
@@ -767,7 +767,7 @@ static void __devexit divas_remove_one(struct pci_dev *pdev)
 /* --------------------------------------------------------------------------
    Driver Load / Startup
    -------------------------------------------------------------------------- */
-static int DIVA_INIT_FUNCTION divas_init(void)
+static int __init divas_init(void)
 {
 	char tmprev[50];
 	int ret = 0;
@@ -831,7 +831,7 @@ out:
 /* --------------------------------------------------------------------------
    Driver Unload
    -------------------------------------------------------------------------- */
-static void DIVA_EXIT_FUNCTION divas_exit(void)
+static void __exit divas_exit(void)
 {
 	pci_unregister_driver(&diva_pci_driver);
 	remove_divas_proc();
diff --git a/drivers/isdn/hardware/eicon/idifunc.c b/drivers/isdn/hardware/eicon/idifunc.c
index d153e3c..fef6586 100644
--- a/drivers/isdn/hardware/eicon/idifunc.c
+++ b/drivers/isdn/hardware/eicon/idifunc.c
@@ -133,7 +133,7 @@ static void um_remove_card(DESCRIPTOR *d)
 /*
  * remove all adapter
  */
-static void DIVA_EXIT_FUNCTION remove_all_idi_proc(void)
+static void __exit remove_all_idi_proc(void)
 {
 	udiva_card *card;
 	diva_os_spin_lock_magic_t old_irql;
@@ -181,7 +181,7 @@ static void *didd_callback(void *context, DESCRIPTOR *adapter,
 /*
  * connect DIDD
  */
-static int DIVA_INIT_FUNCTION connect_didd(void)
+static int __init connect_didd(void)
 {
 	int x = 0;
 	int dadapter = 0;
@@ -225,7 +225,7 @@ static int DIVA_INIT_FUNCTION connect_didd(void)
 /*
  *  Disconnect from DIDD
  */
-static void DIVA_EXIT_FUNCTION disconnect_didd(void)
+static void __exit disconnect_didd(void)
 {
 	IDI_SYNC_REQ req;
 
@@ -240,7 +240,7 @@ static void DIVA_EXIT_FUNCTION disconnect_didd(void)
 /*
  * init
  */
-int DIVA_INIT_FUNCTION idifunc_init(void)
+int __init idifunc_init(void)
 {
 	diva_os_initialize_spin_lock(&ll_lock, "idifunc");
 
@@ -260,7 +260,7 @@ int DIVA_INIT_FUNCTION idifunc_init(void)
 /*
  * finit
  */
-void DIVA_EXIT_FUNCTION idifunc_finit(void)
+void __exit idifunc_finit(void)
 {
 	diva_user_mode_idi_finit();
 	disconnect_didd();
diff --git a/drivers/isdn/hardware/eicon/mntfunc.c b/drivers/isdn/hardware/eicon/mntfunc.c
index d607260..1cd9aff 100644
--- a/drivers/isdn/hardware/eicon/mntfunc.c
+++ b/drivers/isdn/hardware/eicon/mntfunc.c
@@ -72,7 +72,7 @@ static void *didd_callback(void *context, DESCRIPTOR *adapter,
 /*
  * connect to didd
  */
-static int DIVA_INIT_FUNCTION connect_didd(void)
+static int __init connect_didd(void)
 {
 	int x = 0;
 	int dadapter = 0;
@@ -114,7 +114,7 @@ static int DIVA_INIT_FUNCTION connect_didd(void)
 /*
  * disconnect from didd
  */
-static void DIVA_EXIT_FUNCTION disconnect_didd(void)
+static void __exit disconnect_didd(void)
 {
 	IDI_SYNC_REQ req;
 
@@ -300,7 +300,7 @@ int maint_read_write(void __user *buf, int count)
 /*
  *  init
  */
-int DIVA_INIT_FUNCTION mntfunc_init(int *buffer_length, void **buffer,
+int __init mntfunc_init(int *buffer_length, void **buffer,
 				    unsigned long diva_dbg_mem)
 {
 	if (*buffer_length < 64) {
@@ -348,7 +348,7 @@ int DIVA_INIT_FUNCTION mntfunc_init(int *buffer_length, void **buffer,
 /*
  *  exit
  */
-void DIVA_EXIT_FUNCTION mntfunc_finit(void)
+void __exit mntfunc_finit(void)
 {
 	void *buffer;
 	int i = 100;
diff --git a/drivers/isdn/hardware/eicon/platform.h b/drivers/isdn/hardware/eicon/platform.h
index 7331c3b..b2edb75 100644
--- a/drivers/isdn/hardware/eicon/platform.h
+++ b/drivers/isdn/hardware/eicon/platform.h
@@ -38,9 +38,6 @@
 #define DIVA_NO_DEBUGLIB
 #endif
 
-#define DIVA_INIT_FUNCTION  __init
-#define DIVA_EXIT_FUNCTION  __exit
-
 #define DIVA_USER_MODE_CARD_CONFIG 1
 #define	USE_EXTENDED_DEBUGS 1
 

^ permalink raw reply related

* Re: [PATCH V2 1/2] bonding support for IPv6 transmit hashing
From: Jay Vosburgh @ 2012-04-24 22:51 UTC (permalink / raw)
  To: John; +Cc: netdev
In-Reply-To: <4F8E0A8D.9080803@8192.net>

John <linux@8192.net> wrote:

	In principle, I think this is a good idea, but the patch has
some (mostly style) issues, and does not apply to net-next (you don't
say what tree it is based on).

	Can you address the comments noted below, and repost against the
current net-next?

>--- a/drivers/net/bonding/bond_main.c	2012-03-18 16:15:34.000000000 -0700
>+++ b/drivers/net/bonding/bond_main.c	2012-04-14 20:23:26.000000000 -0700
>@@ -3352,56 +3352,87 @@
> /*---------------------------- Hashing Policies -----------------------------*/
>
> /*
>+ * Hash for the output device based upon layer 2 data
>+ */
>+static int bond_xmit_hash_policy_l2(struct sk_buff *skb, int count)
>+{
>+	struct ethhdr *data = (struct ethhdr *)skb->data;
>+
>+	if (skb_headlen(skb) >= 6)
>+		return (data->h_dest[5] ^ data->h_source[5]) % count;

	Can skb_headlen ever be less than 6 here?  And why >= 6, anyway?
The offset of ->h_source[5] from skb->data is 11.

>+
>+	return 0;
>+}
>+
>+/*
>  * Hash for the output device based upon layer 2 and layer 3 data. If
>- * the packet is not IP mimic bond_xmit_hash_policy_l2()
>+ * the packet is not IP, fall back on bond_xmit_hash_policy_l2()
>  */
> static int bond_xmit_hash_policy_l23(struct sk_buff *skb, int count)
> {
> 	struct ethhdr *data = (struct ethhdr *)skb->data;
>-	struct iphdr *iph = ip_hdr(skb);
>
>-	if (skb->protocol == htons(ETH_P_IP)) {
>+	if (skb->protocol == htons(ETH_P_IP) &&
>+		skb_network_header_len(skb) >= sizeof(struct iphdr)) {
>+		struct iphdr *iph = ip_hdr(skb);
> 		return ((ntohl(iph->saddr ^ iph->daddr) & 0xffff) ^
> 			(data->h_dest[5] ^ data->h_source[5])) % count;
>+	} else if (skb->protocol == htons(ETH_P_IPV6) &&
>+		skb_network_header_len(skb) >= sizeof(struct ipv6hdr)) {
>+		struct ipv6hdr *ipv6h = ipv6_hdr(skb);
>+		u32 v6hash =

	I personally prefer having variables declared at the head of the
function, not inside inner blocks, but if you're going to do this, there
needs to be a blank line after the declaration.

>+			(ipv6h->saddr.s6_addr32[1] ^ ipv6h->daddr.s6_addr32[1]) ^
>+			(ipv6h->saddr.s6_addr32[2] ^ ipv6h->daddr.s6_addr32[2]) ^
>+			(ipv6h->saddr.s6_addr32[3] ^ ipv6h->daddr.s6_addr32[3]);
>+		v6hash = (v6hash >> 16) ^ (v6hash >> 8) ^ v6hash;
>+		return (v6hash ^ data->h_dest[5] ^ data->h_source[5]) % count;
> 	}
>
>-	return (data->h_dest[5] ^ data->h_source[5]) % count;
>+	return bond_xmit_hash_policy_l2(skb, count);
> }
>
> /*
>  * Hash for the output device based upon layer 3 and layer 4 data. If
>  * the packet is a frag or not TCP or UDP, just use layer 3 data.  If it is
>- * altogether not IP, mimic bond_xmit_hash_policy_l2()
>+ * altogether not IP, fall back on bond_xmit_hash_policy_l2()
>  */
> static int bond_xmit_hash_policy_l34(struct sk_buff *skb, int count)
> {
>-	struct ethhdr *data = (struct ethhdr *)skb->data;
>-	struct iphdr *iph = ip_hdr(skb);
>-	__be16 *layer4hdr = (__be16 *)((u32 *)iph + iph->ihl);
>-	int layer4_xor = 0;
>+	u32 layer4_xor = 0;
>
> 	if (skb->protocol == htons(ETH_P_IP)) {
>+		struct iphdr *iph = ip_hdr(skb);
> 		if (!ip_is_fragment(iph) &&

	Blank line after iph declaration.

>-		    (iph->protocol == IPPROTO_TCP ||
>-		     iph->protocol == IPPROTO_UDP)) {
>+			(iph->protocol == IPPROTO_TCP ||
>+			iph->protocol == IPPROTO_UDP)) {
>+			__be16 *layer4hdr = (__be16 *)((u32 *)iph + iph->ihl);

	Blank line afret the layer4hdr declaration.

>+			if (iph->ihl * sizeof(u32) + sizeof(__be16) * 2 >
>+				skb_headlen(skb) - skb_network_offset(skb)) goto SHORT_HEADER;

	The goto needs to be on a separate line.

> 			layer4_xor = ntohs((*layer4hdr ^ *(layer4hdr + 1)));
>+		} else if (skb_network_header_len(skb) < sizeof(struct iphdr)) {
>+			goto SHORT_HEADER;
> 		}
> 		return (layer4_xor ^
> 			((ntohl(iph->saddr ^ iph->daddr)) & 0xffff)) % count;
>-
>+	} else if (skb->protocol == htons(ETH_P_IPV6)) {
>+		struct ipv6hdr *ipv6h = ipv6_hdr(skb);
>+		if (ipv6h->nexthdr == IPPROTO_TCP || ipv6h->nexthdr == IPPROTO_UDP) {

	Blank line after ipv6h.

>+			__be16 *layer4hdrv6 = (__be16 *)((u8 *)ipv6h + sizeof(struct ipv6hdr));
>+			if (sizeof(struct ipv6hdr) + sizeof(__be16) * 2 >
>+				skb_headlen(skb) - skb_network_offset(skb)) goto SHORT_HEADER;

	The goto goes on a separate line.

>+			layer4_xor = (*layer4hdrv6 ^ *(layer4hdrv6 + 1));
>+		} else if (skb_network_header_len(skb) < sizeof(struct ipv6hdr)) {
>+			goto SHORT_HEADER;
>+		}
>+		layer4_xor ^=
>+			(ipv6h->saddr.s6_addr32[1] ^ ipv6h->daddr.s6_addr32[1]) ^
>+			(ipv6h->saddr.s6_addr32[2] ^ ipv6h->daddr.s6_addr32[2]) ^
>+			(ipv6h->saddr.s6_addr32[3] ^ ipv6h->daddr.s6_addr32[3]);
>+		return ((layer4_xor >> 16) ^ (layer4_xor >> 8) ^ layer4_xor) % count;
> 	}
>
>-	return (data->h_dest[5] ^ data->h_source[5]) % count;
>-}
>-
>-/*
>- * Hash for the output device based upon layer 2 data
>- */
>-static int bond_xmit_hash_policy_l2(struct sk_buff *skb, int count)
>-{
>-	struct ethhdr *data = (struct ethhdr *)skb->data;
>-
>-	return (data->h_dest[5] ^ data->h_source[5]) % count;
>+	SHORT_HEADER:

	This label should be lower case, and not indented at all.

	-J


>+	return bond_xmit_hash_policy_l2(skb, count);
> }
>
> /*-------------------------- Device entry points ----------------------------*/

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

^ permalink raw reply

* Re: restoring IP multicast addresses when restarting the interface.
From: Flavio Leitner @ 2012-04-24 22:19 UTC (permalink / raw)
  To: David Stevens; +Cc: David Miller, herbert, netdev
In-Reply-To: <OFB8B0B17A.86602877-ON882579EA.00741868-882579EA.0075F72B@us.ibm.com>

On Tue, 24 Apr 2012 14:28:33 -0700
David Stevens <dlstevens@us.ibm.com> wrote:

> netdev-owner@vger.kernel.org wrote on 04/24/2012 02:00:04 PM:
> 
> > From: David Miller <davem@davemloft.net>
> 
> > >> I don't see why they should disappear when the interface goes
> > >> down and then comes back up since these ultimately come from
> > >> application sockets which continue to exist after a down/up.
> > > 
> > > Yeah, but that's not how things used to work before, so my
> > > question is if the kernel should be responsible for keeping
> > > the subscription or the application.
> > > 
> > > If the admin puts down the interface and remove the module,
> > > for instance, then the multicast subscription is gone.
> > > Should the application monitor for that then?
> > > 
> > > David? Any thoughts?
> > 
> > David Stevens at IBM and Herbert at the current multicast
> > experts, so I will defer to them.
> 
> I think restoring the hardware multicast filter on device up
> is useful, desirable and not a bug. :-)

Alright, thanks everyone!
fbl

^ permalink raw reply

* Message
From: Timothy Hull @ 2012-04-24 21:58 UTC (permalink / raw)

In-Reply-To: <133462921.3194601335304615453.JavaMail.root@zim-store03.web.westnet.com.au>

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



Attached in a PDF file are details of a business proposal.
Regards,
Timothy Hull


[-- Attachment #2: Partner.pdf --]
[-- Type: application/pdf, Size: 95985 bytes --]

^ permalink raw reply

* Re: [PATCH net-next] ipv6: RTAX_FEATURE_ALLFRAG causes inefficient TCP segment sizing
From: Maciej Żenczykowski @ 2012-04-24 21:51 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Tore Anderson, David Miller, netdev, Tom Herbert
In-Reply-To: <CANP3RGfhc+4FjB152r58AP66oZQmj0oY-Zh=9qNn9OABCXPwbw@mail.gmail.com>

> I just don't see what not decreasing mtu below 1280 buys us.

In particular it seems like it would be a good idea to drop tcp mss...

^ permalink raw reply

* Re: My e1000e GBE card is eating all port 623 pks
From: Jesse Brandeburg @ 2012-04-24 21:51 UTC (permalink / raw)
  To: Joakim Tjernlund; +Cc: netdev
In-Reply-To: <OF8F83D7F0.387F6CCE-ONC12579E7.0049B885-C12579E7.004A4356@transmode.se>

On Sat, 21 Apr 2012 15:31:09 +0200
Joakim Tjernlund <joakim.tjernlund@transmode.se> wrote:
 
> Looks like port 623 is some mgmt protocol and our e1000e boards are eating these
> pkgs and this trips NIS, finger and yptest hangs for a long time before timing out and
> moving on.
> 
> Is there a way to tell the network stack not to eat port 623 pkgs or
> have NIS not to use port 623?

I think you might be looking for something like portreserve 
(see man portreserve)

Jesse

^ permalink raw reply

* Re: [PATCH net-next] ipv6: RTAX_FEATURE_ALLFRAG causes inefficient TCP segment sizing
From: Maciej Żenczykowski @ 2012-04-24 21:50 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Tore Anderson, David Miller, netdev, Tom Herbert
In-Reply-To: <1335298228.5205.168.camel@edumazet-glaptop>

On Tue, Apr 24, 2012 at 1:10 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Tue, 2012-04-24 at 12:49 -0700, Maciej Żenczykowski wrote:
>> Why do we refuse to set ipv6 mtu's below 1280?
>> how is what we do any better?
>
> I guess you didnt read Tore use case.
>
> Thats the standard : http://tools.ietf.org/html/rfc2460#section-5
>
>   In response to an IPv6 packet that is sent to an IPv4 destination
>   (i.e., a packet that undergoes translation from IPv6 to IPv4), the
>   originating IPv6 node may receive an ICMP Packet Too Big message
>   reporting a Next-Hop MTU less than 1280.  In that case, the IPv6 node
>   is not required to reduce the size of subsequent packets to less than

... is not required to reduce...

but doesn't that mean it _may_ reduce anyway?
and thus make life easier on the guy who will have to fragment (and
thus he won't have to fragment).

[of course you'd still need to lose 8 bytes for the frag header...]

>   1280, but must include a Fragment header in those packets so that the
>   IPv6-to-IPv4 translating router can obtain a suitable Identification
>   value to use in resulting IPv4 fragments.  Note that this means the
>   payload may have to be reduced to 1232 octets (1280 minus 40 for the
>   IPv6 header and 8 for the Fragment header), and smaller still if
>   additional extension headers are used.

I just don't see what not decreasing mtu below 1280 buys us.

^ permalink raw reply

* Re: restoring IP multicast addresses when restarting the interface.
From: David Stevens @ 2012-04-24 21:28 UTC (permalink / raw)
  To: David Miller; +Cc: fbl, herbert, netdev, netdev-owner
In-Reply-To: <20120424.170004.1811627706738841106.davem@davemloft.net>

netdev-owner@vger.kernel.org wrote on 04/24/2012 02:00:04 PM:

> From: David Miller <davem@davemloft.net>

> >> I don't see why they should disappear when the interface goes
> >> down and then comes back up since these ultimately come from
> >> application sockets which continue to exist after a down/up.
> > 
> > Yeah, but that's not how things used to work before, so my
> > question is if the kernel should be responsible for keeping
> > the subscription or the application.
> > 
> > If the admin puts down the interface and remove the module,
> > for instance, then the multicast subscription is gone.
> > Should the application monitor for that then?
> > 
> > David? Any thoughts?
> 
> David Stevens at IBM and Herbert at the current multicast
> experts, so I will defer to them.

I think restoring the hardware multicast filter on device up
is useful, desirable and not a bug. :-)

                                                +-DLS

^ permalink raw reply

* Re: restoring IP multicast addresses when restarting the interface.
From: David Miller @ 2012-04-24 21:00 UTC (permalink / raw)
  To: fbl; +Cc: herbert, netdev
In-Reply-To: <20120424153023.0913956e@asterix.rh>

From: Flavio Leitner <fbl@redhat.com>
Date: Tue, 24 Apr 2012 15:30:23 -0300

> On Mon, 23 Apr 2012 22:14:25 +1000
> Herbert Xu <herbert@gondor.hengli.com.au> wrote:
> 
>> On Fri, Apr 20, 2012 at 09:25:33PM -0300, Flavio Leitner wrote:
>> >
>> > Although the new behavior seems nice and save some user space
>> > work, I think it was unintentional and likely to be a bug.
>> > 
>> > What you guys think?
>> 
>> Are you talking about multicast subscriptions on the interface?
>  
> Yes.
> 
>> I don't see why they should disappear when the interface goes
>> down and then comes back up since these ultimately come from
>> application sockets which continue to exist after a down/up.
> 
> Yeah, but that's not how things used to work before, so my
> question is if the kernel should be responsible for keeping
> the subscription or the application.
> 
> If the admin puts down the interface and remove the module,
> for instance, then the multicast subscription is gone.
> Should the application monitor for that then?
> 
> David? Any thoughts?

David Stevens at IBM and Herbert at the current multicast
experts, so I will defer to them.

^ permalink raw reply

* [PATCH net-next] ipv6: call consume_skb() in frag/reassembly
From: Eric Dumazet @ 2012-04-24 20:17 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

From: Eric Dumazet <edumazet@google.com>

Some kfree_skb() calls should be replaced by consume_skb() to avoid
drop_monitor/dropwatch false positives.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv6/ip6_output.c |    4 ++--
 net/ipv6/reassembly.c |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index b7ca461..b347062 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -210,7 +210,7 @@ int ip6_xmit(struct sock *sk, struct sk_buff *skb, struct flowi6 *fl6,
 				kfree_skb(skb);
 				return -ENOBUFS;
 			}
-			kfree_skb(skb);
+			consume_skb(skb);
 			skb = skb2;
 			skb_set_owner_w(skb, sk);
 		}
@@ -889,7 +889,7 @@ slow_path:
 	}
 	IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)),
 		      IPSTATS_MIB_FRAGOKS);
-	kfree_skb(skb);
+	consume_skb(skb);
 	return err;
 
 fail:
diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
index 36e04cf..54c5d2b 100644
--- a/net/ipv6/reassembly.c
+++ b/net/ipv6/reassembly.c
@@ -433,7 +433,7 @@ static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev,
 		skb_morph(head, fq->q.fragments);
 		head->next = fq->q.fragments->next;
 
-		kfree_skb(fq->q.fragments);
+		consume_skb(fq->q.fragments);
 		fq->q.fragments = head;
 	}
 

^ permalink raw reply related

* Re: [PATCH net-next] ipv6: RTAX_FEATURE_ALLFRAG causes inefficient TCP segment sizing
From: Eric Dumazet @ 2012-04-24 20:10 UTC (permalink / raw)
  To: Maciej Żenczykowski; +Cc: Tore Anderson, David Miller, netdev, Tom Herbert
In-Reply-To: <CANP3RGd0mV8RcH1zCLBg+N9sgPGbiynhOSfoUAYpcMgYsvzeNA@mail.gmail.com>

On Tue, 2012-04-24 at 12:49 -0700, Maciej Żenczykowski wrote:
> Why do we refuse to set ipv6 mtu's below 1280?
> how is what we do any better?

I guess you didnt read Tore use case.

Thats the standard : http://tools.ietf.org/html/rfc2460#section-5

   In response to an IPv6 packet that is sent to an IPv4 destination
   (i.e., a packet that undergoes translation from IPv6 to IPv4), the
   originating IPv6 node may receive an ICMP Packet Too Big message
   reporting a Next-Hop MTU less than 1280.  In that case, the IPv6 node
   is not required to reduce the size of subsequent packets to less than
   1280, but must include a Fragment header in those packets so that the
   IPv6-to-IPv4 translating router can obtain a suitable Identification
   value to use in resulting IPv4 fragments.  Note that this means the
   payload may have to be reduced to 1232 octets (1280 minus 40 for the
   IPv6 header and 8 for the Fragment header), and smaller still if
   additional extension headers are used.

^ permalink raw reply

* Re: BCM5720
From: Matt Carlson @ 2012-04-24 20:06 UTC (permalink / raw)
  To: Mahesh Bandewar; +Cc: Matt Carlson, Michael Chan, linux-netdev
In-Reply-To: <CAF2d9jgpfV4rod95LdhicyO5bD_9Umsnhev+UwaT03EMyJGxug@mail.gmail.com>

On Tue, Apr 24, 2012 at 01:00:12PM -0700, Mahesh Bandewar wrote:
> Hi,
> 
> The PCI device ID for this was removed sometime in the past. Was there any
> reason behind this (am I missing anything)?

The device ID that was removed was for a preproduction device that was
never released.  There now exists a new device under the same name, but
it has a different device ID.  If you have a BCM5720 device, it should
work with the latest kernels.

^ permalink raw reply

* Re: [PATCH net-next] ipv6: RTAX_FEATURE_ALLFRAG causes inefficient TCP segment sizing
From: Maciej Żenczykowski @ 2012-04-24 19:49 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Tore Anderson, David Miller, netdev, Tom Herbert
In-Reply-To: <1335289058.5205.165.camel@edumazet-glaptop>

Why do we refuse to set ipv6 mtu's below 1280?
how is what we do any better?

On Tue, Apr 24, 2012 at 10:37 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> Quoting Tore Anderson from :
> https://bugzilla.kernel.org/show_bug.cgi?id=42572
>
> When RTAX_FEATURE_ALLFRAG is set on a route, the effective TCP segment
> size does not take into account the size of the IPv6 Fragmentation
> header that needs to be included in outbound packets, causing every
> transmitted TCP segment to be fragmented across two IPv6 packets, the
> latter of which will only contain 8 bytes of actual payload.
>
> RTAX_FEATURE_ALLFRAG is typically set on a route in response to
> receving a ICMPv6 Packet Too Big message indicating a Path MTU of less
> than 1280 bytes. 1280 bytes is the minimum IPv6 MTU, however ICMPv6
> PTBs with MTU < 1280 are still valid, in particular when an IPv6
> packet is sent to an IPv4 destination through a stateless translator.
> Any ICMPv4 Need To Fragment packets originated from the IPv4 part of
> the path will be translated to ICMPv6 PTB which may then indicate an
> MTU of less than 1280.
>
> The Linux kernel refuses to reduce the effective MTU to anything below
> 1280 bytes, instead it sets it to exactly 1280 bytes, and
> RTAX_FEATURE_ALLFRAG is also set. However, the TCP segment size appears
> to be set to 1240 bytes (1280 Path MTU - 40 bytes of IPv6 header),
> instead of 1232 (additionally taking into account the 8 bytes required
> by the IPv6 Fragmentation extension header).
>
> This in turn results in rather inefficient transmission, as every
> transmitted TCP segment now is split in two fragments containing
> 1232+8 bytes of payload.
>
> After this patch, all the outgoing packets that includes a
> Fragmentation header all are "atomic" or "non-fragmented" fragments,
> i.e., they both have Offset=0 and More Fragments=0.
>
> With help from David S. Miller
>
> Reported-by: Tore Anderson <tore@fud.no>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Maciej Żenczykowski <maze@google.com>
> Cc: Tom Herbert <therbert@google.com>
> Tested-by: Tore Anderson <tore@fud.no>
> ---
>  include/net/inet_connection_sock.h |    1 +
>  include/net/tcp.h                  |    4 ++--
>  net/ipv4/tcp_output.c              |   19 +++++++++++++++++--
>  net/ipv6/tcp_ipv6.c                |    1 +
>  4 files changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/include/net/inet_connection_sock.h b/include/net/inet_connection_sock.h
> index 46c9e2c..7d83f90 100644
> --- a/include/net/inet_connection_sock.h
> +++ b/include/net/inet_connection_sock.h
> @@ -45,6 +45,7 @@ struct inet_connection_sock_af_ops {
>                                      struct dst_entry *dst);
>        struct inet_peer *(*get_peer)(struct sock *sk, bool *release_it);
>        u16         net_header_len;
> +       u16         net_frag_header_len;
>        u16         sockaddr_len;
>        int         (*setsockopt)(struct sock *sk, int level, int optname,
>                                  char __user *optval, unsigned int optlen);
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index fc880e9..0fb84de 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -544,8 +544,8 @@ extern int tcp_read_sock(struct sock *sk, read_descriptor_t *desc,
>
>  extern void tcp_initialize_rcv_mss(struct sock *sk);
>
> -extern int tcp_mtu_to_mss(const struct sock *sk, int pmtu);
> -extern int tcp_mss_to_mtu(const struct sock *sk, int mss);
> +extern int tcp_mtu_to_mss(struct sock *sk, int pmtu);
> +extern int tcp_mss_to_mtu(struct sock *sk, int mss);
>  extern void tcp_mtup_init(struct sock *sk);
>  extern void tcp_valid_rtt_meas(struct sock *sk, u32 seq_rtt);
>
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 7b7cf38..834e89f 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -1150,7 +1150,7 @@ int tcp_trim_head(struct sock *sk, struct sk_buff *skb, u32 len)
>  }
>
>  /* Calculate MSS. Not accounting for SACKs here.  */
> -int tcp_mtu_to_mss(const struct sock *sk, int pmtu)
> +int tcp_mtu_to_mss(struct sock *sk, int pmtu)
>  {
>        const struct tcp_sock *tp = tcp_sk(sk);
>        const struct inet_connection_sock *icsk = inet_csk(sk);
> @@ -1161,6 +1161,14 @@ int tcp_mtu_to_mss(const struct sock *sk, int pmtu)
>         */
>        mss_now = pmtu - icsk->icsk_af_ops->net_header_len - sizeof(struct tcphdr);
>
> +       /* IPv6 adds a frag_hdr in case RTAX_FEATURE_ALLFRAG is set */
> +       if (icsk->icsk_af_ops->net_frag_header_len) {
> +               const struct dst_entry *dst = __sk_dst_get(sk);
> +
> +               if (dst && dst_allfrag(dst))
> +                       mss_now -= icsk->icsk_af_ops->net_frag_header_len;
> +       }
> +
>        /* Clamp it (mss_clamp does not include tcp options) */
>        if (mss_now > tp->rx_opt.mss_clamp)
>                mss_now = tp->rx_opt.mss_clamp;
> @@ -1179,7 +1187,7 @@ int tcp_mtu_to_mss(const struct sock *sk, int pmtu)
>  }
>
>  /* Inverse of above */
> -int tcp_mss_to_mtu(const struct sock *sk, int mss)
> +int tcp_mss_to_mtu(struct sock *sk, int mss)
>  {
>        const struct tcp_sock *tp = tcp_sk(sk);
>        const struct inet_connection_sock *icsk = inet_csk(sk);
> @@ -1190,6 +1198,13 @@ int tcp_mss_to_mtu(const struct sock *sk, int mss)
>              icsk->icsk_ext_hdr_len +
>              icsk->icsk_af_ops->net_header_len;
>
> +       /* IPv6 adds a frag_hdr in case RTAX_FEATURE_ALLFRAG is set */
> +       if (icsk->icsk_af_ops->net_frag_header_len) {
> +               const struct dst_entry *dst = __sk_dst_get(sk);
> +
> +               if (dst && dst_allfrag(dst))
> +                       mtu += icsk->icsk_af_ops->net_frag_header_len;
> +       }
>        return mtu;
>  }
>
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index cdbf292..57b2109 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -1778,6 +1778,7 @@ static const struct inet_connection_sock_af_ops ipv6_specific = {
>        .syn_recv_sock     = tcp_v6_syn_recv_sock,
>        .get_peer          = tcp_v6_get_peer,
>        .net_header_len    = sizeof(struct ipv6hdr),
> +       .net_frag_header_len = sizeof(struct frag_hdr),
>        .setsockopt        = ipv6_setsockopt,
>        .getsockopt        = ipv6_getsockopt,
>        .addr2sockaddr     = inet6_csk_addr2sockaddr,
>
>



-- 
Maciej A. Żenczykowski
Kernel Networking Developer @ Google
1600 Amphitheatre Parkway, Mountain View, CA 94043
tel: +1 (650) 253-0062

^ 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