Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 1/2] be2net: Copyright notice change. Update to Emulex instead of ServerEngines
From: Ajit Khaparde @ 2011-03-16 18:20 UTC (permalink / raw)
  To: netdev


Signed-off-by: Ajit Khaparde <ajit.khaparde@emulex.com>
---
 drivers/net/benet/be.h         |   10 +++++-----
 drivers/net/benet/be_cmds.c    |   10 +++++-----
 drivers/net/benet/be_cmds.h    |   10 +++++-----
 drivers/net/benet/be_ethtool.c |   10 +++++-----
 drivers/net/benet/be_hw.h      |   10 +++++-----
 drivers/net/benet/be_main.c    |   10 +++++-----
 6 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/drivers/net/benet/be.h b/drivers/net/benet/be.h
index 62af707..7721699 100644
--- a/drivers/net/benet/be.h
+++ b/drivers/net/benet/be.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2005 - 2010 ServerEngines
+ * Copyright (C) 2005 - 2011 Emulex
  * All rights reserved.
  *
  * This program is free software; you can redistribute it and/or
@@ -8,11 +8,11 @@
  * Public License is included in this distribution in the file called COPYING.
  *
  * Contact Information:
- * linux-drivers@serverengines.com
+ * linux-drivers@emulex.com
  *
- * ServerEngines
- * 209 N. Fair Oaks Ave
- * Sunnyvale, CA 94085
+ * Emulex
+ * 3333 Susan Street
+ * Costa Mesa, CA 92626
  */
 
 #ifndef BE_H
diff --git a/drivers/net/benet/be_cmds.c b/drivers/net/benet/be_cmds.c
index e1124c8..5a4a87e 100644
--- a/drivers/net/benet/be_cmds.c
+++ b/drivers/net/benet/be_cmds.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2005 - 2010 ServerEngines
+ * Copyright (C) 2005 - 2011 Emulex
  * All rights reserved.
  *
  * This program is free software; you can redistribute it and/or
@@ -8,11 +8,11 @@
  * Public License is included in this distribution in the file called COPYING.
  *
  * Contact Information:
- * linux-drivers@serverengines.com
+ * linux-drivers@emulex.com
  *
- * ServerEngines
- * 209 N. Fair Oaks Ave
- * Sunnyvale, CA 94085
+ * Emulex
+ * 3333 Susan Street
+ * Costa Mesa, CA 92626
  */
 
 #include "be.h"
diff --git a/drivers/net/benet/be_cmds.h b/drivers/net/benet/be_cmds.h
index e41fcba..4f254cf 100644
--- a/drivers/net/benet/be_cmds.h
+++ b/drivers/net/benet/be_cmds.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2005 - 2010 ServerEngines
+ * Copyright (C) 2005 - 2011 Emulex
  * All rights reserved.
  *
  * This program is free software; you can redistribute it and/or
@@ -8,11 +8,11 @@
  * Public License is included in this distribution in the file called COPYING.
  *
  * Contact Information:
- * linux-drivers@serverengines.com
+ * linux-drivers@emulex.com
  *
- * ServerEngines
- * 209 N. Fair Oaks Ave
- * Sunnyvale, CA 94085
+ * Emulex
+ * 3333 Susan Street
+ * Costa Mesa, CA 92626
  */
 
 /*
diff --git a/drivers/net/benet/be_ethtool.c b/drivers/net/benet/be_ethtool.c
index 6e5e433..aac248f 100644
--- a/drivers/net/benet/be_ethtool.c
+++ b/drivers/net/benet/be_ethtool.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2005 - 2010 ServerEngines
+ * Copyright (C) 2005 - 2011 Emulex
  * All rights reserved.
  *
  * This program is free software; you can redistribute it and/or
@@ -8,11 +8,11 @@
  * Public License is included in this distribution in the file called COPYING.
  *
  * Contact Information:
- * linux-drivers@serverengines.com
+ * linux-drivers@emulex.com
  *
- * ServerEngines
- * 209 N. Fair Oaks Ave
- * Sunnyvale, CA 94085
+ * Emulex
+ * 3333 Susan Street
+ * Costa Mesa, CA 92626
  */
 
 #include "be.h"
diff --git a/drivers/net/benet/be_hw.h b/drivers/net/benet/be_hw.h
index e06aa15..d4344a0 100644
--- a/drivers/net/benet/be_hw.h
+++ b/drivers/net/benet/be_hw.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2005 - 2010 ServerEngines
+ * Copyright (C) 2005 - 2011 Emulex
  * All rights reserved.
  *
  * This program is free software; you can redistribute it and/or
@@ -8,11 +8,11 @@
  * Public License is included in this distribution in the file called COPYING.
  *
  * Contact Information:
- * linux-drivers@serverengines.com
+ * linux-drivers@emulex.com
  *
- * ServerEngines
- * 209 N. Fair Oaks Ave
- * Sunnyvale, CA 94085
+ * Emulex
+ * 3333 Susan Street
+ * Costa Mesa, CA 92626
  */
 
 /********* Mailbox door bell *************/
diff --git a/drivers/net/benet/be_main.c b/drivers/net/benet/be_main.c
index 5e15006..a71163f 100644
--- a/drivers/net/benet/be_main.c
+++ b/drivers/net/benet/be_main.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2005 - 2010 ServerEngines
+ * Copyright (C) 2005 - 2011 Emulex
  * All rights reserved.
  *
  * This program is free software; you can redistribute it and/or
@@ -8,11 +8,11 @@
  * Public License is included in this distribution in the file called COPYING.
  *
  * Contact Information:
- * linux-drivers@serverengines.com
+ * linux-drivers@emulex.com
  *
- * ServerEngines
- * 209 N. Fair Oaks Ave
- * Sunnyvale, CA 94085
+ * Emulex
+ * 3333 Susan Street
+ * Costa Mesa, CA 92626
  */
 
 #include "be.h"
-- 
1.7.1


^ permalink raw reply related

* [PATCH net-next 2/2] be2net: Bump up the version number
From: Ajit Khaparde @ 2011-03-16 18:21 UTC (permalink / raw)
  To: netdev


Signed-off-by: Ajit Khaparde <ajit.khaparde@emulex.com>
---
 drivers/net/benet/be.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/benet/be.h b/drivers/net/benet/be.h
index 7721699..f803c58 100644
--- a/drivers/net/benet/be.h
+++ b/drivers/net/benet/be.h
@@ -33,7 +33,7 @@
 
 #include "be_hw.h"
 
-#define DRV_VER			"2.103.175u"
+#define DRV_VER			"4.0.100u"
 #define DRV_NAME		"be2net"
 #define BE_NAME			"ServerEngines BladeEngine2 10Gbps NIC"
 #define BE3_NAME		"ServerEngines BladeEngine3 10Gbps NIC"
-- 
1.7.1


^ permalink raw reply related

* Re: net-next-2.6 status...
From: David Miller @ 2011-03-16 18:17 UTC (permalink / raw)
  To: bhutchings
  Cc: jpirko, netdev, linux-wireless, netfilter-devel, eric.dumazet,
	linville, kaber
In-Reply-To: <1300299219.2811.7.camel@bwh-desktop>

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Wed, 16 Mar 2011 18:13:39 +0000

> Though, in this case, the priv_flags are not in any way visible to
> user-space, so it would be quite hard to find a use for them!

There are pieces of infrastructure, such as scripts that turn header
file visible macro values into values usable in other languages such
as perl, that do that wholesale for entire header files and therefore
expect these individual values to be there.

I'm not going to be the person who breaks such things.

^ permalink raw reply

* Re: [PATCH] net: 2.6.38 Compile Failure e1000e Module
From: David Miller @ 2011-03-16 18:15 UTC (permalink / raw)
  To: randy.dunlap; +Cc: frank.peters, linux-kernel, netdev
In-Reply-To: <20110316082105.c911bb1c.randy.dunlap@oracle.com>

From: Randy Dunlap <randy.dunlap@oracle.com>
Date: Wed, 16 Mar 2011 08:21:05 -0700

> Hi,
> Please test this patch:
> 
> 
> From: Randy Dunlap <randy.dunlap@oracle.com>
> 
> e1000e needs to select CRC32 for CRC support.
> This patch fixes the build error:
> ERROR: "crc32_le" [drivers/net/e1000e/e1000e.ko] undefined!
> 
> Reported-by: Frank Peters <frank.peters@comcast.net>
> Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>

Randy, I applied Eric Dumazet's copy of this fix only because
his submission preceded your's in terms of time of submission :-)

Thanks!

^ permalink raw reply

* Re: 2.6.38 Compile Failure e1000e Module
From: David Miller @ 2011-03-16 18:15 UTC (permalink / raw)
  To: eric.dumazet
  Cc: frank.peters, linux-kernel, bruce.w.allan, jeffrey.t.kirsher,
	netdev
In-Reply-To: <1300288617.3202.10.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 16 Mar 2011 16:16:57 +0100

> [PATCH] e1000e: fix kconfig for crc32 dependency
> 
> ERROR: "crc32_le" [drivers/net/e1000e/e1000e.ko] undefined!
> 
> Reported-by: Frank Peters <frank.peters@comcast.net>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Bruce Allan <bruce.w.allan@intel.com>
> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

Applied, thanks Eric.

^ permalink raw reply

* Re: net-next-2.6 status...
From: Ben Hutchings @ 2011-03-16 18:13 UTC (permalink / raw)
  To: David Miller
  Cc: jpirko, netdev, linux-wireless, netfilter-devel, eric.dumazet,
	linville, kaber
In-Reply-To: <20110316.104236.104065822.davem@davemloft.net>

On Wed, 2011-03-16 at 10:42 -0700, David Miller wrote:
> From: Jiri Pirko <jpirko@redhat.com>
> Date: Wed, 16 Mar 2011 09:40:23 +0100
> 
> > Wed, Mar 16, 2011 at 08:38:03AM CET, jpirko@redhat.com wrote:
> >>Wed, Mar 16, 2011 at 08:02:51AM CET, davem@davemloft.net wrote:
> >>>From: Jiri Pirko <jpirko@redhat.com>
> >>>Date: Wed, 16 Mar 2011 07:50:09 +0100
> >>>
> >>>>>Jiri, I know there is your patch set there, but I think you and Changli
> >>>>>still need to go back and forth one more time wrt. orig_dev semantics.
> >>>>>Since you have been posting this patch set for some time I'm still
> >>>>>willing to apply it for this merge window, but please make haste and
> >>>>>work out the remaining discussion.  Thank you.
> >>>> 
> >>>> Dave, you can apply the rest of the series and leave only the first
> >>>> patch (af_packet) out. There's no dependency. We will figure out things
> >>>> around origdev later.
> >>>
> >>>I was about to do that but I've found other problems.
> >>>
> >>>You cannot make the modifications you make to linux/if.h, those
> >>>interface flags are visible to userspace.
> >>
> >>What do you suggest? To remove unused flags and leave gaps there or to
> >>not to remove the flags at all?
> > 
> > Well, the following comment:
> > <quote>
> > /* Private (from user) interface flags (netdevice->priv_flags). */
> > </quote>
> > leads me to think that these flags should not be used by userspace.
> > So maybe it may not be problem to change those values.
> 
> Comments don't block user applications from using defines we expose
> to them.  The person who was wise enough to write that comment should
> have also been wise enough to add appropriate __KERNEL__ protection
> to the definitions.

Though, in this case, the priv_flags are not in any way visible to
user-space, so it would be quite hard to find a use for them!

> They were not, therefore we are stuck with them forever.

So should we also expose struct net_device_stats again?  During the
introduction to 64-bit stats I changed and un-exported the definition,
then later had to revert the definition but left it un-exported.  This
broke compilation of iproute2, though in the process it revealed an
existing bug: <http://thread.gmane.org/gmane.linux.network/169764>.

Ben.

-- 
Ben Hutchings, Senior Software 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

* Re: [PATCH 00/01] netfilter: netfilter fix for net-next
From: David Miller @ 2011-03-16 18:12 UTC (permalink / raw)
  To: kaber; +Cc: netfilter-devel, netdev
In-Reply-To: <1300297111-28116-1-git-send-email-kaber@trash.net>

From: kaber@trash.net
Date: Wed, 16 Mar 2011 18:38:30 +0100

> following is a single netfilter fix from Thomas for net-next. The new
> AUDIT target is supposed to work with ebtables, however it returns
> a xtables verdict, which are unfortunately incompatible with ebtables
> verdicts.
> 
> Please apply or pull from:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/kaber/nf-next-2.6.git master

I'll pull this, thanks Patrick.


^ permalink raw reply

* Re: net-next-2.6 status...
From: David Miller @ 2011-03-16 18:00 UTC (permalink / raw)
  To: jpirko
  Cc: netdev, linux-wireless, netfilter-devel, eric.dumazet, linville,
	kaber
In-Reply-To: <20110316175642.GB2867@psychotron.redhat.com>

From: Jiri Pirko <jpirko@redhat.com>
Date: Wed, 16 Mar 2011 18:56:44 +0100

> So would you leave out if.h hunk out or should I repost?

What makes you think it's a good idea for me to have to do more work?

I have the entire merge window and thousands of other patches to worry
about...

^ permalink raw reply

* Re: net-next-2.6 status...
From: Jiri Pirko @ 2011-03-16 17:56 UTC (permalink / raw)
  To: David Miller
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netfilter-devel-u79uwXL29TY76Z2rM5mHXA,
	eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
	linville-2XuSBdqkA4R54TAoqtyWWQ, kaber-dcUjhNyLwpNeoWH0uzbU5w
In-Reply-To: <20110316.103857.226773145.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>

Wed, Mar 16, 2011 at 06:38:57PM CET, davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org wrote:
>From: Jiri Pirko <jpirko-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>Date: Wed, 16 Mar 2011 08:38:03 +0100
>
>> Wed, Mar 16, 2011 at 08:02:51AM CET, davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org wrote:
>>>From: Jiri Pirko <jpirko-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>>Date: Wed, 16 Mar 2011 07:50:09 +0100
>>>
>>>>>Jiri, I know there is your patch set there, but I think you and Changli
>>>>>still need to go back and forth one more time wrt. orig_dev semantics.
>>>>>Since you have been posting this patch set for some time I'm still
>>>>>willing to apply it for this merge window, but please make haste and
>>>>>work out the remaining discussion.  Thank you.
>>>> 
>>>> Dave, you can apply the rest of the series and leave only the first
>>>> patch (af_packet) out. There's no dependency. We will figure out things
>>>> around origdev later.
>>>
>>>I was about to do that but I've found other problems.
>>>
>>>You cannot make the modifications you make to linux/if.h, those
>>>interface flags are visible to userspace.
>> 
>> What do you suggest? To remove unused flags and leave gaps there or to
>> not to remove the flags at all?
>
>You can't remove them, otherwise if it happened to be reference in the
>sources to some app out there, it will start to fail to compile.
>
>This is pretty obvious stuff, I hope :-)

Ok :( So would you leave out if.h hunk out or should I repost?

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: build breakage due to br_multicast.c referencing ipv6_dev_get_saddr()
From: David Miller @ 2011-03-16 17:49 UTC (permalink / raw)
  To: JBeulich; +Cc: linus.luessing, shemminger, bridge, netdev
In-Reply-To: <4D80BC5B0200007800036D85@vpn.id2.novell.com>

From: "Jan Beulich" <JBeulich@novell.com>
Date: Wed, 16 Mar 2011 12:34:19 +0000

> With BRIDGE=y and IPV6=m commit
> fe29ec41aaa51902aebd63658dfb04fe6fea8be5 ("bridge: Use IPv6
> link-local address for multicast listener queries") causes the build to
> break.

Fixed by:

commit dcbcdf22f500ac6e4ec06485341024739b9dc241
Author: Randy Dunlap <randy.dunlap@oracle.com>
Date:   Thu Mar 10 13:45:57 2011 -0800

    net: bridge builtin vs. ipv6 modular
    
    When configs BRIDGE=y and IPV6=m, this build error occurs:
    
    br_multicast.c:(.text+0xa3341): undefined reference to `ipv6_dev_get_saddr'
    
    BRIDGE_IGMP_SNOOPING is boolean; if it were tristate, then adding
    	depends on IPV6 || IPV6=n
    to BRIDGE_IGMP_SNOOPING would be a good fix.  As it is currently,
    making BRIDGE depend on the IPV6 config works.
    
    Reported-by: Patrick Schaaf <netdev@bof.de>
    Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/net/bridge/Kconfig b/net/bridge/Kconfig
index 9190ae4..6dee7bf 100644
--- a/net/bridge/Kconfig
+++ b/net/bridge/Kconfig
@@ -6,6 +6,7 @@ config BRIDGE
 	tristate "802.1d Ethernet Bridging"
 	select LLC
 	select STP
+	depends on IPV6 || IPV6=n
 	---help---
 	  If you say Y here, then your Linux box will be able to act as an
 	  Ethernet bridge, which means that the different Ethernet segments it

^ permalink raw reply related

* Re: build breakage due to br_multicast.c referencing ipv6_dev_get_saddr()
From: David Miller @ 2011-03-16 17:49 UTC (permalink / raw)
  To: shemminger; +Cc: JBeulich, linus.luessing, bridge, netdev
In-Reply-To: <20110316082441.45db3018@nehalam>

From: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Wed, 16 Mar 2011 08:24:41 -0700

> On Wed, 16 Mar 2011 12:34:19 +0000
> "Jan Beulich" <JBeulich@novell.com> wrote:
> 
>> With BRIDGE=y and IPV6=m commit
>> fe29ec41aaa51902aebd63658dfb04fe6fea8be5 ("bridge: Use IPv6
>> link-local address for multicast listener queries") causes the build to
>> break.
> 
> Rather than continue with the config games, lets just make the necessary
> ipv6 pieces accessible.

You can't Stephen, ipv6_dev_get_saddr() requires access to the actual ipv6
device state, that means you have to pull in the entire ipv6 stack in because
there are dependencies all the way down into the routing code.

We added a Kconfig fix to cure this specific problem, which made it
into 2.6.38-final, so I don't understand why Jan is even seeing this,
it's supposed to force BRIDGE modular if IPV6 is modular:

commit dcbcdf22f500ac6e4ec06485341024739b9dc241
Author: Randy Dunlap <randy.dunlap@oracle.com>
Date:   Thu Mar 10 13:45:57 2011 -0800

    net: bridge builtin vs. ipv6 modular
    
    When configs BRIDGE=y and IPV6=m, this build error occurs:
    
    br_multicast.c:(.text+0xa3341): undefined reference to `ipv6_dev_get_saddr'
    
    BRIDGE_IGMP_SNOOPING is boolean; if it were tristate, then adding
    	depends on IPV6 || IPV6=n
    to BRIDGE_IGMP_SNOOPING would be a good fix.  As it is currently,
    making BRIDGE depend on the IPV6 config works.
    
    Reported-by: Patrick Schaaf <netdev@bof.de>
    Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/net/bridge/Kconfig b/net/bridge/Kconfig
index 9190ae4..6dee7bf 100644
--- a/net/bridge/Kconfig
+++ b/net/bridge/Kconfig
@@ -6,6 +6,7 @@ config BRIDGE
 	tristate "802.1d Ethernet Bridging"
 	select LLC
 	select STP
+	depends on IPV6 || IPV6=n
 	---help---
 	  If you say Y here, then your Linux box will be able to act as an
 	  Ethernet bridge, which means that the different Ethernet segments it

^ permalink raw reply related

* Re: net-next-2.6 status...
From: David Miller @ 2011-03-16 17:42 UTC (permalink / raw)
  To: jpirko
  Cc: netdev, linux-wireless, netfilter-devel, eric.dumazet, linville,
	kaber
In-Reply-To: <20110316084022.GA2845@psychotron.brq.redhat.com>

From: Jiri Pirko <jpirko@redhat.com>
Date: Wed, 16 Mar 2011 09:40:23 +0100

> Wed, Mar 16, 2011 at 08:38:03AM CET, jpirko@redhat.com wrote:
>>Wed, Mar 16, 2011 at 08:02:51AM CET, davem@davemloft.net wrote:
>>>From: Jiri Pirko <jpirko@redhat.com>
>>>Date: Wed, 16 Mar 2011 07:50:09 +0100
>>>
>>>>>Jiri, I know there is your patch set there, but I think you and Changli
>>>>>still need to go back and forth one more time wrt. orig_dev semantics.
>>>>>Since you have been posting this patch set for some time I'm still
>>>>>willing to apply it for this merge window, but please make haste and
>>>>>work out the remaining discussion.  Thank you.
>>>> 
>>>> Dave, you can apply the rest of the series and leave only the first
>>>> patch (af_packet) out. There's no dependency. We will figure out things
>>>> around origdev later.
>>>
>>>I was about to do that but I've found other problems.
>>>
>>>You cannot make the modifications you make to linux/if.h, those
>>>interface flags are visible to userspace.
>>
>>What do you suggest? To remove unused flags and leave gaps there or to
>>not to remove the flags at all?
> 
> Well, the following comment:
> <quote>
> /* Private (from user) interface flags (netdevice->priv_flags). */
> </quote>
> leads me to think that these flags should not be used by userspace.
> So maybe it may not be problem to change those values.

Comments don't block user applications from using defines we expose
to them.  The person who was wise enough to write that comment should
have also been wise enough to add appropriate __KERNEL__ protection
to the definitions.

They were not, therefore we are stuck with them forever.

^ permalink raw reply

* Re: [PATCH V4] Export ACPI _DSM provided firmware instance number and string name to sysfs
From: Jesse Barnes @ 2011-03-16 17:41 UTC (permalink / raw)
  To: Shyam_Iyer
  Cc: Narendra_K, greg, a.beregalov, linux-next, linux-pci,
	linux-hotplug, netdev, mjg, Matt_Domsch, Charles_Rose,
	Jordan_Hargrave, sfr
In-Reply-To: <DBFB1B45AF80394ABD1C807E9F28D157026585D732@BLRX7MCDC203.AMER.DELL.COM>

On Fri, 11 Mar 2011 03:35:56 +0530
<Shyam_Iyer@Dell.com> wrote:

> 
> > -----Original Message-----
> > From: K, Narendra
> > Sent: Monday, March 07, 2011 3:56 PM
> > To: Greg KH
> > Cc: a.beregalov@gmail.com; linux-next@vger.kernel.org;
> > jbarnes@virtuousgeek.org; linux-pci@vger.kernel.org; linux-
> > hotplug@vger.kernel.org; netdev@vger.kernel.org; mjg@redhat.com;
> > Domsch, Matt; Rose, Charles; Hargrave, Jordan; Iyer, Shyam;
> > sfr@canb.auug.org.au
> > Subject: Re: [PATCH V4] Export ACPI _DSM provided firmware instance
> > number and string name to sysfs
> > 
> > On Tue, Mar 08, 2011 at 01:26:16AM +0530, Greg KH wrote:
> > > On Mon, Mar 07, 2011 at 11:44:52AM -0800, Narendra_K@Dell.com wrote:
> > > > --- a/drivers/pci/pci-label.c
> > > > +++ b/drivers/pci/pci-label.c
> > > > @@ -29,7 +29,9 @@
> > > >  #include <linux/nls.h>
> > > >  #include <linux/acpi.h>
> > > >  #include <linux/pci-acpi.h>
> > > > +#ifdef CONFIG_ACPI
> > > >  #include <acpi/acpi_drivers.h>
> > > > +#endif
> > >
> > > You should never need a #ifdef in a .c file for an include file.  If
> > so,
> > > something is really wrong.
> > 
> > I agree. Also, i realized that the include was not required to address
> > the
> > reported error. Please find the revised patch here.
> > 
> > From: Narendra K <narendra_k@dell.com>
> > Subject: [PATCH] Fix compilation error when CONFIG_ACPI is unset
> > 
> > This patch fixes compilation error descibed below introduced by
> > the commit 6058989bad05b82e78baacce69ec14f27a11b5fd
> > 
> > drivers/pci/pci-label.c: In function ‘pci_create_firmware_label_files’:
> > drivers/pci/pci-label.c:366:2: error: implicit declaration of function
> > ‘device_has_dsm’
> > 
> > Signed-off-by: Narendra K <narendra_k@dell.com>
> > ---
> >  drivers/pci/pci-label.c |    6 ++++++
> >  1 files changed, 6 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/pci/pci-label.c b/drivers/pci/pci-label.c
> > index 824e247..8c80138 100644
> > --- a/drivers/pci/pci-label.c
> > +++ b/drivers/pci/pci-label.c
> > @@ -174,6 +174,12 @@ pci_remove_acpi_index_label_files(struct pci_dev
> > *pdev)
> >  	return -1;
> >  }
> > 
> > +static inline bool
> > +device_has_dsm(struct device *dev)
> > +{
> > +	return false;
> > +}
> > +
> >  #else
> > 
> >  static const char device_label_dsm_uuid[] = {
> > --
> > 1.7.3.1
> > 
> > With regards,
> > Narendra K
> 
> So this works and fixes the additional build failure.
> 
> I tested with CONFIG_ACPI set/unset and with "make allmodconfig"
> 
> Additionally I found that including acpi/apci_drivers.h is not necessary and introduces these warnings..
> 
> The below patch fixes the additional warnigs..
> 
> In file included from drivers/pci/pci-label.c:32:
> include/acpi/acpi_drivers.h:103: warning: ‘struct acpi_device’ declared inside parameter list
> include/acpi/acpi_drivers.h:103: warning: its scope is only this definition or declaration, which is probably not what you want
> include/acpi/acpi_drivers.h:107: warning: ‘struct acpi_pci_root’ declared inside parameter list
> 
> Signed-off-by: Shyam Iyer <shyam_iyer@dell.com>

Ok, I've applied these two fixes, thanks guys.  I hope that's the last
of the issues we'll see with this patch!

-- 
Jesse Barnes, Intel Open Source Technology Center

^ permalink raw reply

* Re: [PATCH 1/2] xfrm: Force a dst refcount before entering the xfrm type handlers
From: David Miller @ 2011-03-16 17:41 UTC (permalink / raw)
  To: steffen.klassert; +Cc: eric.dumazet, netdev
In-Reply-To: <20110316074306.GT31402@secunet.com>

From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Wed, 16 Mar 2011 08:43:06 +0100

> On Wed, Mar 16, 2011 at 12:17:32AM -0700, David Miller wrote:
>> 
>> Steffen we really need to find another way to fix these problems.
>> 
>> We already make way too many expensive atomic operations in these code
>> paths modified by your 3 patches, we should strive to not add new
>> ones.
>> 
> 
> I know that it is exspensive, but we have to take a refcount if
> the crypto layer returns asyncronous. Unfortunately it is too
> late to take the refcount when the crypto layer notifies us about
> that as the skb might be already gone.

We can pass around an atomic_t for the crypto layer to bump when
it decides to take an async path, bump it on entry and upon event
wakeup decrement it.

Actually, a plain atomic_t is a bad idea because we might have to
release the object.  So callbacks with private void pointer arg is
better.

^ permalink raw reply

* Re: net-next-2.6 status...
From: David Miller @ 2011-03-16 17:38 UTC (permalink / raw)
  To: jpirko-H+wXaHxf7aLQT0dZR+AlfA
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netfilter-devel-u79uwXL29TY76Z2rM5mHXA,
	eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
	linville-2XuSBdqkA4R54TAoqtyWWQ, kaber-dcUjhNyLwpNeoWH0uzbU5w
In-Reply-To: <20110316073803.GB2780-YzwxZg+R7et1/kRsl7OVgNvLeJWuRmrY@public.gmane.org>

From: Jiri Pirko <jpirko-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Date: Wed, 16 Mar 2011 08:38:03 +0100

> Wed, Mar 16, 2011 at 08:02:51AM CET, davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org wrote:
>>From: Jiri Pirko <jpirko-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>Date: Wed, 16 Mar 2011 07:50:09 +0100
>>
>>>>Jiri, I know there is your patch set there, but I think you and Changli
>>>>still need to go back and forth one more time wrt. orig_dev semantics.
>>>>Since you have been posting this patch set for some time I'm still
>>>>willing to apply it for this merge window, but please make haste and
>>>>work out the remaining discussion.  Thank you.
>>> 
>>> Dave, you can apply the rest of the series and leave only the first
>>> patch (af_packet) out. There's no dependency. We will figure out things
>>> around origdev later.
>>
>>I was about to do that but I've found other problems.
>>
>>You cannot make the modifications you make to linux/if.h, those
>>interface flags are visible to userspace.
> 
> What do you suggest? To remove unused flags and leave gaps there or to
> not to remove the flags at all?

You can't remove them, otherwise if it happened to be reference in the
sources to some app out there, it will start to fail to compile.

This is pretty obvious stuff, I hope :-)
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: net-next-2.6 status...
From: David Miller @ 2011-03-16 17:38 UTC (permalink / raw)
  To: tgraf-wEGCiKHe2LqWVfeAwA7xHQ
  Cc: jpirko-H+wXaHxf7aLQT0dZR+AlfA, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netfilter-devel-u79uwXL29TY76Z2rM5mHXA,
	eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
	linville-2XuSBdqkA4R54TAoqtyWWQ, kaber-dcUjhNyLwpNeoWH0uzbU5w
In-Reply-To: <20110316084739.GD26145-oJAMn8VJQk8CUdFEqe4BF2D2FQJk+8+b@public.gmane.org>

From: Thomas Graf <tgraf-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
Date: Wed, 16 Mar 2011 04:47:39 -0400

> On Wed, Mar 16, 2011 at 09:40:23AM +0100, Jiri Pirko wrote:
>> >What do you suggest? To remove unused flags and leave gaps there or to
>> >not to remove the flags at all?
>> 
>> Well, the following comment:
>> <quote>
>> /* Private (from user) interface flags (netdevice->priv_flags). */
>> </quote>
>> leads me to think that these flags should not be used by userspace.
>> So maybe it may not be problem to change those values.
> 
> These flags should be moved to linux/netdevice.h

We've had them exposed in a header file without __KERNEL__ protection
for a long time, the cat is therefore out of the bag and it's too late
to just remove them from user visibility as if they were not there.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH 01/01] netfilter ebtables: fix xt_AUDIT to work with ebtables
From: kaber @ 2011-03-16 17:38 UTC (permalink / raw)
  To: davem; +Cc: netfilter-devel, netdev
In-Reply-To: <1300297111-28116-1-git-send-email-kaber@trash.net>

From: Thomas Graf <tgraf@redhat.com>

Even though ebtables uses xtables it still requires targets to
return EBT_CONTINUE instead of XT_CONTINUE. This prevented
xt_AUDIT to work as ebt module.

Upon Jan's suggestion, use a separate struct xt_target for
NFPROTO_BRIDGE having its own target callback returning
EBT_CONTINUE instead of cloning the module.

Signed-off-by: Thomas Graf <tgraf@redhat.com>
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
 net/netfilter/xt_AUDIT.c |   36 +++++++++++++++++++++++++++---------
 1 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/net/netfilter/xt_AUDIT.c b/net/netfilter/xt_AUDIT.c
index 81802d2..363a99e 100644
--- a/net/netfilter/xt_AUDIT.c
+++ b/net/netfilter/xt_AUDIT.c
@@ -19,6 +19,7 @@
 #include <linux/if_arp.h>
 #include <linux/netfilter/x_tables.h>
 #include <linux/netfilter/xt_AUDIT.h>
+#include <linux/netfilter_bridge/ebtables.h>
 #include <net/ipv6.h>
 #include <net/ip.h>
 
@@ -168,6 +169,13 @@ errout:
 	return XT_CONTINUE;
 }
 
+static unsigned int
+audit_tg_ebt(struct sk_buff *skb, const struct xt_action_param *par)
+{
+	audit_tg(skb, par);
+	return EBT_CONTINUE;
+}
+
 static int audit_tg_check(const struct xt_tgchk_param *par)
 {
 	const struct xt_audit_info *info = par->targinfo;
@@ -181,23 +189,33 @@ static int audit_tg_check(const struct xt_tgchk_param *par)
 	return 0;
 }
 
-static struct xt_target audit_tg_reg __read_mostly = {
-	.name		= "AUDIT",
-	.family		= NFPROTO_UNSPEC,
-	.target		= audit_tg,
-	.targetsize	= sizeof(struct xt_audit_info),
-	.checkentry	= audit_tg_check,
-	.me		= THIS_MODULE,
+static struct xt_target audit_tg_reg[] __read_mostly = {
+	{
+		.name		= "AUDIT",
+		.family		= NFPROTO_UNSPEC,
+		.target		= audit_tg,
+		.targetsize	= sizeof(struct xt_audit_info),
+		.checkentry	= audit_tg_check,
+		.me		= THIS_MODULE,
+	},
+	{
+		.name		= "AUDIT",
+		.family		= NFPROTO_BRIDGE,
+		.target		= audit_tg_ebt,
+		.targetsize	= sizeof(struct xt_audit_info),
+		.checkentry	= audit_tg_check,
+		.me		= THIS_MODULE,
+	},
 };
 
 static int __init audit_tg_init(void)
 {
-	return xt_register_target(&audit_tg_reg);
+	return xt_register_targets(audit_tg_reg, ARRAY_SIZE(audit_tg_reg));
 }
 
 static void __exit audit_tg_exit(void)
 {
-	xt_unregister_target(&audit_tg_reg);
+	xt_unregister_targets(audit_tg_reg, ARRAY_SIZE(audit_tg_reg));
 }
 
 module_init(audit_tg_init);
-- 
1.7.2.3


^ permalink raw reply related

* [PATCH 00/01] netfilter: netfilter fix for net-next
From: kaber @ 2011-03-16 17:38 UTC (permalink / raw)
  To: davem; +Cc: netfilter-devel, netdev

Hi Dave,

following is a single netfilter fix from Thomas for net-next. The new
AUDIT target is supposed to work with ebtables, however it returns
a xtables verdict, which are unfortunately incompatible with ebtables
verdicts.

Please apply or pull from:

git://git.kernel.org/pub/scm/linux/kernel/git/kaber/nf-next-2.6.git master

Thanks!


^ permalink raw reply

* Re: build breakage due to br_multicast.c referencing  ipv6_dev_get_saddr()
From: Brian Haley @ 2011-03-16 17:34 UTC (permalink / raw)
  To: Jan Beulich; +Cc: linus.luessing, davem, shemminger, bridge, netdev
In-Reply-To: <4D80BC5B0200007800036D85@vpn.id2.novell.com>

On 03/16/2011 08:34 AM, Jan Beulich wrote:
> With BRIDGE=y and IPV6=m commit
> fe29ec41aaa51902aebd63658dfb04fe6fea8be5 ("bridge: Use IPv6
> link-local address for multicast listener queries") causes the build to
> break.
> 
> Similary, even if both are =m, but ipv6.ko got blacklisted (as is
> happening in various SuSE distros when disabling IPv6), there's
> a runtime problem since bridge.ko then won't load anymore due
> to the missing symbol.

Load the ipv6 module with disable=1, which is why I added it :)

>From Documentation/networking/ipv6.txt:

disable

        Specifies whether to load the IPv6 module, but disable all
        its functionality.  This might be used when another module
        has a dependency on the IPv6 module being loaded, but no
        IPv6 addresses or operations are desired.

        The possible values and their effects are:

        0
                IPv6 is enabled.

                This is the default value.

        1
                IPv6 is disabled.

                No IPv6 addresses will be added to interfaces, and
                it will not be possible to open an IPv6 socket.

                A reboot is required to enable IPv6.

-Brian

^ permalink raw reply

* Re: [PATCH][v3] dev : fix mtu check when TSO is enabled
From: Stephen Hemminger @ 2011-03-16 16:45 UTC (permalink / raw)
  To: Daniel Lezcano; +Cc: David Miller, eric.dumazet, kaber, nightnord, netdev
In-Reply-To: <4D80E302.9010806@free.fr>

On Wed, 16 Mar 2011 17:19:14 +0100
Daniel Lezcano <daniel.lezcano@free.fr> wrote:

> On 03/16/2011 04:35 PM, Stephen Hemminger wrote:
> > On Wed, 16 Mar 2011 14:56:09 +0100
> > Daniel Lezcano<daniel.lezcano@free.fr>  wrote:
> >
> >> On 03/15/2011 07:17 PM, Stephen Hemminger wrote:
> >>> On Tue, 15 Mar 2011 14:57:40 +0100
> >>> Daniel Lezcano<daniel.lezcano@free.fr>   wrote:
> >>>
> >>>> On 03/15/2011 12:59 AM, David Miller wrote:
> >>>>> From: Daniel Lezcano<daniel.lezcano@free.fr>
> >>>>> Date: Mon, 14 Mar 2011 21:39:50 +0100
> >>>>>
> >>>>>> +	len = dev->mtu + dev->hard_header_len + VLAN_HLEN;
> >>>>>> +	if (skb->len<    len)
> >>>>>> +		return true;
> >>>>> This is not a correct translation of the original test:
> >>>>>
> >>>>>> -		     (skb->len>    (dev->mtu + dev->hard_header_len + VLAN_HLEN)))) {
> >>>>> You need to use "<=" in your version, which currently rejects all
> >>>>> full sized frames. :-)
> >>>> Right, thanks.
> >>>>
> >>>>>> +
> >>>>>> +	/* if TSO is enabled, we don't care about the length as the packet
> >>>>>> +	 * could be forwarded without being segmented before
> >>>>>> +	 */
> >>>>>> +	if (skb->dev&&    skb->dev->features&    NETIF_F_TSO)
> >>>>>> +		return true;
> >>>>> I am trying to understand why you aren't simply checking also if this
> >>>>> is a segmented frame?  Perhaps skb_is_gso()&&    device has NETIF_F_TSO
> >>>>> set?
> >>>> Maybe I am misunderstanding but the packet was forwarded by another device.
> >>>> In our case from macvlan:
> >>>>
> >>>> macvlan_start_xmit
> >>>>        macvlan_queue_xmit
> >>>>            dest->forward
> >>>>                dev_skb_forward
> >>>>
> >>>> When we reached dev_skb_forward, that means we passed through
> >>>> dev_hard_start_xmit where the packet was already segmented so we should
> >>>> exit at the first test (skb->len<   len). I don't see the point of adding
> >>>> the skb_is_gso.
> >>>> But maybe I am missing something, can you explain ?
> >>> The macvlan device only has one downstream device (slave).
> >>> If kernel is working properly, macvlan device should have a subset
> >>> of the features of the underlying device
> >> Right, dev->features = lowerdev->features&  MACVLAN_FEATURES
> >>
> >>> and macvlan device should
> >>> have same MTU as underlying device.
> >> Right,
> >>
> >> ...
> >>
> >>    if (!tb[IFLA_MTU])
> >>           dev->mtu = lowerdev->mtu;
> >>
> >> ...
> >>> If the feature/MTU flags
> >>> were correct, then the path calling macvlan should be respecting
> >>> the MTU.
> >> But if the TSO is enabled on the macvlan (inherited from eg e1000), the
> >> packet won't be fragmented to the mtu size no ?
> > That is the responsiblity of the hardware that receives the packet.
> > Macvlan should be passing it through to the lowerdev and since the hardware
> > supports TSO, it will fragment it.
> 
> Ok, but in the case the macvlan is in bridge mode, the dev_skb_forward 
> function will forward the packet (which is not fragmented) to to another 
> macvlan port without going through the hardware driver. In this 
> function, the packet length is checked against the mtu size and of 
> course the packet is dropped in case the lower device support the TSO 
> (if the packet is larger than the mtu size). Dave suggested to check 
> skb_is_gso and against the TSO feature of the macvlan but I don't 
> understand why we should check skb_is_gso too.
> 
> 	if (skb_is_gso(skb)&&  (skb->dev&&  skb->dev->features&  NETIF_F_TSO))
> 		return true;
> 
> 

Then it is up to macvlan to do the same thing as bridge code.

static inline unsigned packet_length(const struct sk_buff *skb)
{
	return skb->len - (skb->protocol == htons(ETH_P_8021Q) ? VLAN_HLEN : 0);
}

int br_dev_queue_push_xmit(struct sk_buff *skb)
{
	/* ip_fragment doesn't copy the MAC header */
	if (nf_bridge_maybe_copy_header(skb) ||
	    (packet_length(skb) > skb->dev->mtu && !skb_is_gso(skb))) {
		kfree_skb(skb);
	} else {
		skb_push(skb, ETH_HLEN);
		dev_queue_xmit(skb);
	}

	return 0;
}

Ps: not sure adding macvlan bridge mode was a good idea, we already have
a working bridge code. And there are too many missing pieces in the macvlan
implementation of bridging



-- 

^ permalink raw reply

* Re: build breakage due to br_multicast.c referencing ipv6_dev_get_saddr()
From: Stephen Hemminger @ 2011-03-16 16:41 UTC (permalink / raw)
  To: Jan Beulich; +Cc: linus.luessing, davem, bridge, netdev
In-Reply-To: <4D80BC5B0200007800036D85@vpn.id2.novell.com>

On Wed, 16 Mar 2011 12:34:19 +0000
"Jan Beulich" <JBeulich@novell.com> wrote:

> With BRIDGE=y and IPV6=m commit
> fe29ec41aaa51902aebd63658dfb04fe6fea8be5 ("bridge: Use IPv6
> link-local address for multicast listener queries") causes the build to
> break.

I have no problem building with 2.6.38 and BRIDGE=y and IPV6=m.
Blacklisting ipv6 module is just self inflicted pain, don't do it.



^ permalink raw reply

* Re: [PATCH][v3] dev : fix mtu check when TSO is enabled
From: Daniel Lezcano @ 2011-03-16 16:19 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David Miller, eric.dumazet, kaber, nightnord, netdev
In-Reply-To: <20110316083510.5b9d8c72@nehalam>

On 03/16/2011 04:35 PM, Stephen Hemminger wrote:
> On Wed, 16 Mar 2011 14:56:09 +0100
> Daniel Lezcano<daniel.lezcano@free.fr>  wrote:
>
>> On 03/15/2011 07:17 PM, Stephen Hemminger wrote:
>>> On Tue, 15 Mar 2011 14:57:40 +0100
>>> Daniel Lezcano<daniel.lezcano@free.fr>   wrote:
>>>
>>>> On 03/15/2011 12:59 AM, David Miller wrote:
>>>>> From: Daniel Lezcano<daniel.lezcano@free.fr>
>>>>> Date: Mon, 14 Mar 2011 21:39:50 +0100
>>>>>
>>>>>> +	len = dev->mtu + dev->hard_header_len + VLAN_HLEN;
>>>>>> +	if (skb->len<    len)
>>>>>> +		return true;
>>>>> This is not a correct translation of the original test:
>>>>>
>>>>>> -		     (skb->len>    (dev->mtu + dev->hard_header_len + VLAN_HLEN)))) {
>>>>> You need to use "<=" in your version, which currently rejects all
>>>>> full sized frames. :-)
>>>> Right, thanks.
>>>>
>>>>>> +
>>>>>> +	/* if TSO is enabled, we don't care about the length as the packet
>>>>>> +	 * could be forwarded without being segmented before
>>>>>> +	 */
>>>>>> +	if (skb->dev&&    skb->dev->features&    NETIF_F_TSO)
>>>>>> +		return true;
>>>>> I am trying to understand why you aren't simply checking also if this
>>>>> is a segmented frame?  Perhaps skb_is_gso()&&    device has NETIF_F_TSO
>>>>> set?
>>>> Maybe I am misunderstanding but the packet was forwarded by another device.
>>>> In our case from macvlan:
>>>>
>>>> macvlan_start_xmit
>>>>        macvlan_queue_xmit
>>>>            dest->forward
>>>>                dev_skb_forward
>>>>
>>>> When we reached dev_skb_forward, that means we passed through
>>>> dev_hard_start_xmit where the packet was already segmented so we should
>>>> exit at the first test (skb->len<   len). I don't see the point of adding
>>>> the skb_is_gso.
>>>> But maybe I am missing something, can you explain ?
>>> The macvlan device only has one downstream device (slave).
>>> If kernel is working properly, macvlan device should have a subset
>>> of the features of the underlying device
>> Right, dev->features = lowerdev->features&  MACVLAN_FEATURES
>>
>>> and macvlan device should
>>> have same MTU as underlying device.
>> Right,
>>
>> ...
>>
>>    if (!tb[IFLA_MTU])
>>           dev->mtu = lowerdev->mtu;
>>
>> ...
>>> If the feature/MTU flags
>>> were correct, then the path calling macvlan should be respecting
>>> the MTU.
>> But if the TSO is enabled on the macvlan (inherited from eg e1000), the
>> packet won't be fragmented to the mtu size no ?
> That is the responsiblity of the hardware that receives the packet.
> Macvlan should be passing it through to the lowerdev and since the hardware
> supports TSO, it will fragment it.

Ok, but in the case the macvlan is in bridge mode, the dev_skb_forward 
function will forward the packet (which is not fragmented) to to another 
macvlan port without going through the hardware driver. In this 
function, the packet length is checked against the mtu size and of 
course the packet is dropped in case the lower device support the TSO 
(if the packet is larger than the mtu size). Dave suggested to check 
skb_is_gso and against the TSO feature of the macvlan but I don't 
understand why we should check skb_is_gso too.

	if (skb_is_gso(skb)&&  (skb->dev&&  skb->dev->features&  NETIF_F_TSO))
		return true;



^ permalink raw reply

* Re: [PATCH] tcp: avoid cwnd moderation in undo
From: Carsten Wolff @ 2011-03-16 16:18 UTC (permalink / raw)
  To: Yuchung Cheng
  Cc: David Miller, Ilpo Jarvinen, Nandita Dukkipati, netdev,
	Alexander Zimmermann
In-Reply-To: <AANLkTinzkJKb9EtOys2EOW8SDnORdEOT=+UMNBACTJJe@mail.gmail.com>

[-- Attachment #1: Type: Text/Plain, Size: 5367 bytes --]

Hi again,

On Wednesday 16 March 2011, Yuchung Cheng wrote:
> On Tue, Mar 15, 2011 at 3:07 AM, Carsten Wolff <carsten@wolffcarsten.de> 
wrote:
> > Hi,
> > 
> > On Monday 14 March 2011, Yuchung Cheng wrote:
> > > On Mon, Mar 14, 2011 at 3:06 AM, Carsten Wolff
> > > <carsten@wolffcarsten.de>
> > 
> > wrote:
> > > In the presence of reordering, cwnd is already moderated in Disorder
> > > state before
> > >  entering the (false) recovery.
> > 
> > Sure, cwnd moderation to in_flight + 1 segment is applied in disorder
> > state,
> 
> it's in_flight + 3 usually. the moderation first happens
> tcp_try_to_open() instead of tcp_cwnd_down()

In disorder state, tcp_try_to_open() calls tcp_cwnd_down() which clamps cwnd 
to in_flight + 1 for dupacks (where tcp_packets_in_flight() is not to be 
confused with the IN_FLIGHT variable in IETF documents, which is called 
packets_out in Linux ...). Otherwise, Linux would be violating RFC3042, which 
allows to send one SMSS of data on each dupack before recovery (actually, just 
the first two, but since the DupThresh can be larger than 3 in linux, it 
extends Limited Transmit to more than just the first two dupacks). This is 
mostly equivalent to the aggressive variant of extended limited transmit in 
RFC4653.

> > because this is implementing a form of extended limited transmit.
> > Nevertheless, after a reordering event that caused a spurious fast
> > retransmit, there can be an undo of congestion state changes (either
> > after recovery or interrupting recovery, depending on the options
> > enabled in the connection). I just wanted to point out, that the
> > moderation step happening upon an undo may allow a larger burst, if a
> > previous reordering event was detected and caused tp->reordering to be
> > increased.
> 
> Your point is that cwnd should be moderated on reordering (in undo or
> other events). Point taken.
>  My point is that cwnd does not need to be moderated on false
> recoveries. Do you agree?
> To implement your design, tcp_update_reordering should do
> tcp_cwnd_moderation().
> To implement my point, the moderations should be avoided in undo
> operations.
> 
> The two aren't in conflict. But there are cases that have both undo
> and reordering.
> Are we on the same page?

Unfortunately, no. ;-) My point is, that cwnd should be moderated when the 
congestion state changes are undone after a spurious recovery has been 
detected. Reordering is only one possible reason for a false recovery. And I 
stick to that point because of the thoughts I pointed out in my mail to john, 
i.e. undo typically leading to exceptionally large segment bursts.

As for cwnd moderation upon the detection of a reordering event (that's a 
different thing at a differnt point in time than detection of a false 
recovery!): This wouldn't make sense to me. The detection of the reordering 
event together with a metric that measures the extent of the reordering can be 
used to try and prevent false recoverys in future reordering events, by 
delaying the congestion reaction (i.e. fast retransmit) then.

Reordering can be a cause of spurious recovery. But undo mechanisms and 
mechanisms to prevent false recovery(s) are orthogonal.

Your patch touches all undos, while reordering is just an example for a cause 
of false recovery.

> > > > More importantly, the prior ssthresh is restored and not affected by
> > > > moderation. This means, if moderation reduces cwnd to a small value,
> > > > then cwnd < ssthresh and TCP will quickly slow-start back to the
> > > > previous state, without sending a big burst of segments.
> > 
> > This is actually the more important point, because it means that the
> > moderation does not negate the effects of the undo operation, as
> > suggested by your patch-description.
> 
> It's a double-edge sword. Why slow-start if there is no real loss?

Its a timing thing. I mean, it is an undo operation: the harm has been done, 
some opportunity to send new data has been lost. Trying to send all that data 
at once now without an ACK-clock will cause more harm when buffers are under 
pressure. The undo operation should not try to make up for lost opportunity, 
only try to reduce further loss of opportunity to send new data. For this, the 
segment bursts have to be moderated.

> It
> hurts short
> request-response type traffic performance badly b/c each undo makes cwnd =
> 3.
> 
> > False fast retransmits are mostly caused by reordering, spurious RTOs can
> > also be caused by delay variations that do not exhibit reordering. Your
> > patch touches all cases of spurious events. Anyway, I just mentioned
> > reordering, because it is the event in which Linux already allows larger
> > bursts of size tp->reordering in the moderation function (i.e.
> > tp->reordering might be increased). It's also not important to me if the
> > undo is happening duringor after recovery, the important question is, if
> > burst protection in general is an important goal, or not (and I think
> > it's there for a reason).
> 
> I am hoping my previous explanation make sense to you (these two points are
> not in conflict).

I hope the same for my explanations. :-)

Cheers
Carsten
-- 
           /\-´-/\
          (  @ @  )
________o0O___^___O0o________

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

^ permalink raw reply

* Re: [PATCH][v3] dev : fix mtu check when TSO is enabled
From: Stephen Hemminger @ 2011-03-16 15:35 UTC (permalink / raw)
  To: Daniel Lezcano; +Cc: David Miller, eric.dumazet, kaber, nightnord, netdev
In-Reply-To: <4D80C179.8000900@free.fr>

On Wed, 16 Mar 2011 14:56:09 +0100
Daniel Lezcano <daniel.lezcano@free.fr> wrote:

> On 03/15/2011 07:17 PM, Stephen Hemminger wrote:
> > On Tue, 15 Mar 2011 14:57:40 +0100
> > Daniel Lezcano<daniel.lezcano@free.fr>  wrote:
> >
> >> On 03/15/2011 12:59 AM, David Miller wrote:
> >>> From: Daniel Lezcano<daniel.lezcano@free.fr>
> >>> Date: Mon, 14 Mar 2011 21:39:50 +0100
> >>>
> >>>> +	len = dev->mtu + dev->hard_header_len + VLAN_HLEN;
> >>>> +	if (skb->len<   len)
> >>>> +		return true;
> >>> This is not a correct translation of the original test:
> >>>
> >>>> -		     (skb->len>   (dev->mtu + dev->hard_header_len + VLAN_HLEN)))) {
> >>> You need to use "<=" in your version, which currently rejects all
> >>> full sized frames. :-)
> >> Right, thanks.
> >>
> >>>> +
> >>>> +	/* if TSO is enabled, we don't care about the length as the packet
> >>>> +	 * could be forwarded without being segmented before
> >>>> +	 */
> >>>> +	if (skb->dev&&   skb->dev->features&   NETIF_F_TSO)
> >>>> +		return true;
> >>> I am trying to understand why you aren't simply checking also if this
> >>> is a segmented frame?  Perhaps skb_is_gso()&&   device has NETIF_F_TSO
> >>> set?
> >> Maybe I am misunderstanding but the packet was forwarded by another device.
> >> In our case from macvlan:
> >>
> >> macvlan_start_xmit
> >>       macvlan_queue_xmit
> >>           dest->forward
> >>               dev_skb_forward
> >>
> >> When we reached dev_skb_forward, that means we passed through
> >> dev_hard_start_xmit where the packet was already segmented so we should
> >> exit at the first test (skb->len<  len). I don't see the point of adding
> >> the skb_is_gso.
> >> But maybe I am missing something, can you explain ?
> > The macvlan device only has one downstream device (slave).
> > If kernel is working properly, macvlan device should have a subset
> > of the features of the underlying device
> 
> Right, dev->features = lowerdev->features & MACVLAN_FEATURES
> 
> > and macvlan device should
> > have same MTU as underlying device.
> 
> Right,
> 
> ...
> 
>   if (!tb[IFLA_MTU])
>          dev->mtu = lowerdev->mtu;
> 
> ...
> > If the feature/MTU flags
> > were correct, then the path calling macvlan should be respecting
> > the MTU.
> 
> But if the TSO is enabled on the macvlan (inherited from eg e1000), the 
> packet won't be fragmented to the mtu size no ?

That is the responsiblity of the hardware that receives the packet.
Macvlan should be passing it through to the lowerdev and since the hardware
supports TSO, it will fragment it.


-- 

^ permalink raw reply

* Re: build breakage due to br_multicast.c referencing ipv6_dev_get_saddr()
From: Stephen Hemminger @ 2011-03-16 15:24 UTC (permalink / raw)
  To: Jan Beulich; +Cc: linus.luessing, davem, bridge, netdev
In-Reply-To: <4D80BC5B0200007800036D85@vpn.id2.novell.com>

On Wed, 16 Mar 2011 12:34:19 +0000
"Jan Beulich" <JBeulich@novell.com> wrote:

> With BRIDGE=y and IPV6=m commit
> fe29ec41aaa51902aebd63658dfb04fe6fea8be5 ("bridge: Use IPv6
> link-local address for multicast listener queries") causes the build to
> break.

Rather than continue with the config games, lets just make the necessary
ipv6 pieces accessible.

-- 

^ 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