Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v3] tcp: ioctl type SIOCOUTQNSD returns amount of data not sent
From: David Miller @ 2011-03-10 22:16 UTC (permalink / raw)
  To: hagen
  Cc: sledz, netdev, linux-kernel, alan, kuznet, pekkas, jmorris,
	yoshfuji, kaber, m.schuknecht
In-Reply-To: <20110310221205.GF3121@nuttenaction>

From: Hagen Paul Pfeifer <hagen@jauu.net>
Date: Thu, 10 Mar 2011 23:12:05 +0100

> * David Miller | 2011-03-09 14:09:51 [-0800]:
> 
>>From: Steffen Sledz <sledz@dresearch.de>
>>Date: Sun,  6 Mar 2011 20:59:55 +0100
>>
>>> From: Mario Schuknecht <m.schuknecht@dresearch.de>
>>> 
>>> In contrast to SIOCOUTQ which returns the amount of data sent
>>> but not yet acknowledged plus data not yet sent this patch only
>>> returns the data not sent.
>>> 
>>> For various methods of live streaming bitrate control it may
>>> be helpful to know how much data are in the tcp outqueue are
>>> not sent yet.
>>> 
>>> Signed-off-by: Mario Schuknecht <m.schuknecht@dresearch.de>
>>> Signed-off-by: Steffen Sledz <sledz@dresearch.de>
>>
>>Applied, thanks.
> 
> A little bit late - but why not using struct tcp_info for that? The
> information is already provided via getsockopt(TCP_INFO).

It's pretty heavy handed to copy that entire TCP_INFO struct into
userspace during every I/O sequence the application makes.

^ permalink raw reply

* Re: [Patch]ariadne: remove redundant NULL check
From: David Miller @ 2011-03-10 22:12 UTC (permalink / raw)
  To: j223yang; +Cc: netdev, linux-kernel
In-Reply-To: <20110310172031.GA10734@asset.uwaterloo.ca>

From: j223yang@asset.uwaterloo.ca
Date: Thu, 10 Mar 2011 12:20:31 -0500

> Simply remove redundant 'dev' NULL check. 
> 
> Signed-off-by: Jinqiu Yang<crindy646@gmail.com>

This patch was corrupted by your email client, it turned TAB
characters into spaces which makes the patch unusable.

Also your signoff is badly formed, a space is missing between
your name and the "<" that begins your email address.

^ permalink raw reply

* Re: [PATCH v3] tcp: ioctl type SIOCOUTQNSD returns amount of data not sent
From: Hagen Paul Pfeifer @ 2011-03-10 22:12 UTC (permalink / raw)
  To: David Miller
  Cc: sledz, netdev, linux-kernel, alan, kuznet, pekkas, jmorris,
	yoshfuji, kaber, m.schuknecht
In-Reply-To: <20110309.140951.232741403.davem@davemloft.net>

* David Miller | 2011-03-09 14:09:51 [-0800]:

>From: Steffen Sledz <sledz@dresearch.de>
>Date: Sun,  6 Mar 2011 20:59:55 +0100
>
>> From: Mario Schuknecht <m.schuknecht@dresearch.de>
>> 
>> In contrast to SIOCOUTQ which returns the amount of data sent
>> but not yet acknowledged plus data not yet sent this patch only
>> returns the data not sent.
>> 
>> For various methods of live streaming bitrate control it may
>> be helpful to know how much data are in the tcp outqueue are
>> not sent yet.
>> 
>> Signed-off-by: Mario Schuknecht <m.schuknecht@dresearch.de>
>> Signed-off-by: Steffen Sledz <sledz@dresearch.de>
>
>Applied, thanks.

A little bit late - but why not using struct tcp_info for that? The
information is already provided via getsockopt(TCP_INFO).

Hagen

^ permalink raw reply

* RE: [PATCH V4] Export ACPI _DSM provided firmware instance number and string name to sysfs
From: Shyam_Iyer @ 2011-03-10 22:05 UTC (permalink / raw)
  To: Narendra_K, greg
  Cc: a.beregalov, linux-next, jbarnes, linux-pci, linux-hotplug,
	netdev, mjg, Matt_Domsch, Charles_Rose, Jordan_Hargrave, sfr
In-Reply-To: <20110307211552.GA11016@fedora14-r610.oslab.blr.amer.dell.com>


> -----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>
---
 drivers/pci/pci-label.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/pci/pci-label.c b/drivers/pci/pci-label.c
index bf4ebee..488b0ce 100644
--- a/drivers/pci/pci-label.c
+++ b/drivers/pci/pci-label.c
@@ -29,7 +29,6 @@
 #include <linux/nls.h>
 #include <linux/acpi.h>
 #include <linux/pci-acpi.h>
-#include <acpi/acpi_drivers.h>
 #include <acpi/acpi_bus.h>
 #include "pci.h"

-- 
1.7.1

^ permalink raw reply related

* Re: [PATCH] ip6ip6: autoload ip6 tunnel
From: David Miller @ 2011-03-10 22:00 UTC (permalink / raw)
  To: shemminger; +Cc: netdev, segoon, mjt, kees.cook, jmorris
In-Reply-To: <20110310134319.25b1b901@nehalam>

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Thu, 10 Mar 2011 13:43:19 -0800

> Add necessary alias to autoload ip6ip6 tunnel module.
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> 
> ---
> Patch is against 2.6.38-rc7 (not net-next).

Yep, understood.  I'll do a merge then apply this, thanks Stephen!

^ permalink raw reply

* [PATCH] ip6ip6: autoload ip6 tunnel
From: Stephen Hemminger @ 2011-03-10 21:43 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, Vasiliy Kulikov, Michael Tokarev, Kees Cook, James Morris

Add necessary alias to autoload ip6ip6 tunnel module.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

---
Patch is against 2.6.38-rc7 (not net-next).

--- a/net/ipv6/ip6_tunnel.c	2011-03-10 13:33:16.613345613 -0800
+++ b/net/ipv6/ip6_tunnel.c	2011-03-10 13:37:32.372100323 -0800
@@ -57,6 +57,7 @@
 MODULE_AUTHOR("Ville Nuorvala");
 MODULE_DESCRIPTION("IPv6 tunneling device");
 MODULE_LICENSE("GPL");
+MODULE_ALIAS_NETDEV("ip6tnl0");
 
 #ifdef IP6_TNL_DEBUG
 #define IP6_TNL_TRACE(x...) printk(KERN_DEBUG "%s:" x "\n", __func__)

^ permalink raw reply

* Re: [PATCH 2.6.38-rc8 build failure] net: bridge builtin vs. ipv6 modular
From: David Miller @ 2011-03-10 21:49 UTC (permalink / raw)
  To: randy.dunlap; +Cc: netdev, shemminger, netdev
In-Reply-To: <20110310132801.4a919df2.randy.dunlap@oracle.com>

From: Randy Dunlap <randy.dunlap@oracle.com>
Date: Thu, 10 Mar 2011 13:28:01 -0800

> This can be fixed in either of 2 ways.  One way is to require that the
> bridge code building depend on how the IPV6 code is built (builtin or modular);
> the other way is to handle the kconfig variations in net/bridge/br_multicast.c --
> but this loses some functionality if IPV6=m and BRIDGE=y.
> 
> I prefer the Kconfig patch over the br_multicast.c patch.
> Both patches are below for consideration.

I definitely prefer the Kconfig variant, although it's unfortunate that we
have a stateful dependency in the bridging multicast code.

IPV4 seems to let us use INADDR_ANY as the source address in the multicast
query, but IPV6 does not allow this (as per RFC 2710).

Anyways, I've applied the Kconfig patch, thanks Randy!

^ permalink raw reply

* [PATCH 2.6.38-rc8 build failure] net: bridge builtin vs. ipv6 modular
From: Randy Dunlap @ 2011-03-10 21:28 UTC (permalink / raw)
  To: Patrick Schaaf, Stephen Hemminger; +Cc: netdev, davem
In-Reply-To: <1299752394.4109.31.camel@lat1>

This can be fixed in either of 2 ways.  One way is to require that the
bridge code building depend on how the IPV6 code is built (builtin or modular);
the other way is to handle the kconfig variations in net/bridge/br_multicast.c --
but this loses some functionality if IPV6=m and BRIDGE=y.

I prefer the Kconfig patch over the br_multicast.c patch.
Both patches are below for consideration.

---
From: Randy Dunlap <randy.dunlap@oracle.com>

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>
Cc: Stephen Hemminger <shemminger@linux-foundation.org>
---
 net/bridge/Kconfig |    1 +
 1 file changed, 1 insertion(+)

--- lnx-2638-rc8.orig/net/bridge/Kconfig
+++ lnx-2638-rc8/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

===== alternate patch =====
From: Randy Dunlap <randy.dunlap@oracle.com>

Handle IPV6 vs. BRIDGE config differences in br_multicast.c.
This allows IPV6 to be modular with bridge code either built-in
or modular, but if IPV6=m and BRIDGE=y, some functionality is
lost.

Built with all 6 combinations of IPV6={ymn} and BRIDGE={ym}.

Reported-by: Patrick Schaaf <netdev@bof.de>
Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
Cc: Stephen Hemminger <shemminger@linux-foundation.org>
---
 net/bridge/br_multicast.c |   37 ++++++++++++++++++++----------------
 1 file changed, 21 insertions(+), 16 deletions(-)

--- lnx-2638-rc8.orig/net/bridge/br_multicast.c
+++ lnx-2638-rc8/net/bridge/br_multicast.c
@@ -10,6 +10,11 @@
  *
  */
 
+#if defined(CONFIG_IPV6) || \
+    (defined(CONFIG_IPV6_MODULE) && defined(CONFIG_BRIDGE_MODULE))
+#define USE_IPV6	1
+#endif
+
 #include <linux/err.h>
 #include <linux/if_ether.h>
 #include <linux/igmp.h>
@@ -24,7 +29,7 @@
 #include <linux/slab.h>
 #include <linux/timer.h>
 #include <net/ip.h>
-#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+#if defined(USE_IPV6)
 #include <net/ipv6.h>
 #include <net/mld.h>
 #include <net/addrconf.h>
@@ -36,7 +41,7 @@
 #define mlock_dereference(X, br) \
 	rcu_dereference_protected(X, lockdep_is_held(&br->multicast_lock))
 
-#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+#if defined(USE_IPV6)
 static inline int ipv6_is_transient_multicast(const struct in6_addr *addr)
 {
 	if (ipv6_addr_is_multicast(addr) && IPV6_ADDR_MC_FLAG_TRANSIENT(addr))
@@ -52,7 +57,7 @@ static inline int br_ip_equal(const stru
 	switch (a->proto) {
 	case htons(ETH_P_IP):
 		return a->u.ip4 == b->u.ip4;
-#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+#if defined(USE_IPV6)
 	case htons(ETH_P_IPV6):
 		return ipv6_addr_equal(&a->u.ip6, &b->u.ip6);
 #endif
@@ -65,7 +70,7 @@ static inline int __br_ip4_hash(struct n
 	return jhash_1word(mdb->secret, (__force u32)ip) & (mdb->max - 1);
 }
 
-#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+#if defined(USE_IPV6)
 static inline int __br_ip6_hash(struct net_bridge_mdb_htable *mdb,
 				const struct in6_addr *ip)
 {
@@ -79,7 +84,7 @@ static inline int br_ip_hash(struct net_
 	switch (ip->proto) {
 	case htons(ETH_P_IP):
 		return __br_ip4_hash(mdb, ip->u.ip4);
-#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+#if defined(USE_IPV6)
 	case htons(ETH_P_IPV6):
 		return __br_ip6_hash(mdb, &ip->u.ip6);
 #endif
@@ -121,7 +126,7 @@ static struct net_bridge_mdb_entry *br_m
 	return br_mdb_ip_get(mdb, &br_dst);
 }
 
-#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+#if defined(USE_IPV6)
 static struct net_bridge_mdb_entry *br_mdb_ip6_get(
 	struct net_bridge_mdb_htable *mdb, const struct in6_addr *dst)
 {
@@ -152,7 +157,7 @@ struct net_bridge_mdb_entry *br_mdb_get(
 	case htons(ETH_P_IP):
 		ip.u.ip4 = ip_hdr(skb)->daddr;
 		break;
-#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+#if defined(USE_IPV6)
 	case htons(ETH_P_IPV6):
 		ipv6_addr_copy(&ip.u.ip6, &ipv6_hdr(skb)->daddr);
 		break;
@@ -411,7 +416,7 @@ out:
 	return skb;
 }
 
-#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+#if defined(USE_IPV6)
 static struct sk_buff *br_ip6_multicast_alloc_query(struct net_bridge *br,
 						    struct in6_addr *group)
 {
@@ -496,7 +501,7 @@ static struct sk_buff *br_multicast_allo
 	switch (addr->proto) {
 	case htons(ETH_P_IP):
 		return br_ip4_multicast_alloc_query(br, addr->u.ip4);
-#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+#if defined(USE_IPV6)
 	case htons(ETH_P_IPV6):
 		return br_ip6_multicast_alloc_query(br, &addr->u.ip6);
 #endif
@@ -773,7 +778,7 @@ static int br_ip4_multicast_add_group(st
 	return br_multicast_add_group(br, port, &br_group);
 }
 
-#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+#if defined(USE_IPV6)
 static int br_ip6_multicast_add_group(struct net_bridge *br,
 				      struct net_bridge_port *port,
 				      const struct in6_addr *group)
@@ -845,7 +850,7 @@ static void br_multicast_send_query(stru
 	br_group.proto = htons(ETH_P_IP);
 	__br_multicast_send_query(br, port, &br_group);
 
-#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+#if defined(USE_IPV6)
 	br_group.proto = htons(ETH_P_IPV6);
 	__br_multicast_send_query(br, port, &br_group);
 #endif
@@ -989,7 +994,7 @@ static int br_ip4_multicast_igmp3_report
 	return err;
 }
 
-#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+#if defined(USE_IPV6)
 static int br_ip6_multicast_mld2_report(struct net_bridge *br,
 					struct net_bridge_port *port,
 					struct sk_buff *skb)
@@ -1185,7 +1190,7 @@ out:
 	return err;
 }
 
-#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+#if defined(USE_IPV6)
 static int br_ip6_multicast_query(struct net_bridge *br,
 				  struct net_bridge_port *port,
 				  struct sk_buff *skb)
@@ -1334,7 +1339,7 @@ static void br_ip4_multicast_leave_group
 	br_multicast_leave_group(br, port, &br_group);
 }
 
-#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+#if defined(USE_IPV6)
 static void br_ip6_multicast_leave_group(struct net_bridge *br,
 					 struct net_bridge_port *port,
 					 const struct in6_addr *group)
@@ -1446,7 +1451,7 @@ err_out:
 	return err;
 }
 
-#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+#if defined(USE_IPV6)
 static int br_multicast_ipv6_rcv(struct net_bridge *br,
 				 struct net_bridge_port *port,
 				 struct sk_buff *skb)
@@ -1583,7 +1588,7 @@ int br_multicast_rcv(struct net_bridge *
 	switch (skb->protocol) {
 	case htons(ETH_P_IP):
 		return br_multicast_ipv4_rcv(br, port, skb);
-#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+#if defined(USE_IPV6)
 	case htons(ETH_P_IPV6):
 		return br_multicast_ipv6_rcv(br, port, skb);
 #endif


^ permalink raw reply

* Re: virtio_net: remove recv refill work?
From: Shirley Ma @ 2011-03-10 21:22 UTC (permalink / raw)
  To: Rusty Russell; +Cc: kvm, netdev, Michael S. Tsirkin
In-Reply-To: <1299791038.25664.213.camel@localhost.localdomain>

Never mind, the refill work is needed only for OOM. It can't be
replaced.

thanks
Shirley


^ permalink raw reply

* Re: [patch net-next-2.6] net: reinject arps into bonding slave instead of master
From: Jiri Pirko @ 2011-03-10 21:05 UTC (permalink / raw)
  To: Nicolas de Pesloüan; +Cc: netdev
In-Reply-To: <4D793845.1060702@gmail.com>

Thu, Mar 10, 2011 at 09:44:53PM CET, nicolas.2p.debian@gmail.com wrote:
>Le 10/03/2011 07:48, Jiri Pirko a écrit :
>>>But for all others setups, where there exist some net_devices before
>>>the "untagging" one, you would face some troubles. For example, with
>>>eth0+eth1 ->  br0 ->  br0.100, you cannot untag before entering
>>>__netif_receive_skb. If you do so, the bridge would receive untagged
>>>frame and if the frame is not for the local host, the bridge would
>>>forward an untagged frame while it is expected to forward a tagged
>>>one. Even if the bridge is in a position to know the frame *was*
>>>tagged, we cannot expect the bridge to do special processing to
>>>handle this situation. Doing so would break layering.
>>
>>I disagree.
>>eth0 ->  untag on early __netif_receive_skb (sets up skb->vlan_tci)
>>            ->rx_handler of bridge
>>->br0 ->  tag is detected by vlan_tx_tag_present()
>>            ->  reinject to __netif_receive_skb with skb->dev == br0.100
>
>For local delivery, is should work.
>
>But if the bridge must forward the frame to another host (bridges are
>designed for such things :-)), it will have to insert the vlan header
>back into the frame. I don't understand how it could work
>automagically in this situation.

See dev_hard_start_xmit and search for vlan :)

^ permalink raw reply

* virtio_net: remove recv refill work?
From: Shirley Ma @ 2011-03-10 21:03 UTC (permalink / raw)
  To: Rusty Russell; +Cc: kvm, netdev, Michael S. Tsirkin

Hello Rusty,

What's the reason to use refill work in receiving path? 

static void refill_work(struct work_struct *work)
{
        struct virtnet_info *vi;
        bool still_empty;

        vi = container_of(work, struct virtnet_info, refill.work);
        napi_disable(&vi->napi);
        still_empty = !try_fill_recv(vi, GFP_KERNEL);
        napi_enable(&vi->napi);

        /* In theory, this can happen: if we don't get any buffers in
         * we will *never* try to fill again. */
        if (still_empty)
                schedule_delayed_work(&vi->refill, HZ/2);
}

It looks more expensive than only refilling the buffers in recv path. It
could cause more guest exits, maybe more RX interrupts. Can we move the
refill work in recv path only?

If we can remove it, I will run some test to compare the difference with
the change.

Thanks
Shirley




^ permalink raw reply

* Re: [patch net-next-2.6] net: reinject arps into bonding slave instead of master
From: Jiri Pirko @ 2011-03-10 20:52 UTC (permalink / raw)
  To: Nicolas de Pesloüan
  Cc: Andy Gospodarek, netdev, davem, shemminger, kaber, fubar,
	eric.dumazet
In-Reply-To: <4D793845.1060702@gmail.com>

Thu, Mar 10, 2011 at 09:44:53PM CET, nicolas.2p.debian@gmail.com wrote:
>Le 10/03/2011 07:48, Jiri Pirko a écrit :
>>>But for all others setups, where there exist some net_devices before
>>>the "untagging" one, you would face some troubles. For example, with
>>>eth0+eth1 ->  br0 ->  br0.100, you cannot untag before entering
>>>__netif_receive_skb. If you do so, the bridge would receive untagged
>>>frame and if the frame is not for the local host, the bridge would
>>>forward an untagged frame while it is expected to forward a tagged
>>>one. Even if the bridge is in a position to know the frame *was*
>>>tagged, we cannot expect the bridge to do special processing to
>>>handle this situation. Doing so would break layering.
>>
>>I disagree.
>>eth0 ->  untag on early __netif_receive_skb (sets up skb->vlan_tci)
>>            ->rx_handler of bridge
>>->br0 ->  tag is detected by vlan_tx_tag_present()
>>            ->  reinject to __netif_receive_skb with skb->dev == br0.100
>
>For local delivery, is should work.
>
>But if the bridge must forward the frame to another host (bridges are
>designed for such things :-)), it will have to insert the vlan header
>back into the frame. I don't understand how it could work
>automagically in this situation.

This works for hw_accel path. Would look the same.

>
>>This way the flow would be very similar to vlan-hw-accel, am I right?
>
>So your point is to remove any 802.1Q header in any ingres frame,
>whatever the local interface setup. Right?
>
>How would this support nested vlan headers?
>
>eth0 -> eth0.100 -> eth0.100.200 -> eth0.100.200.300.
>
>Who will choose eth0.100.200.300 as the last skb->dev?

Hate to think about this more, that's for sure.

>
>>I have following patch in mind. Note it's raw DRAFT.
>
>I need to take some time to review your patch.
>
>In the mean time, what is the status of the whole patch series? Can we expect an ACK from someone?

It was posponed by davem. I plan to solve vlans first, then repost the
whole series (so that bond recv_probe would see vlan arps)
>
>	Nicolas.

^ permalink raw reply

* Re: [patch net-next-2.6] net: reinject arps into bonding slave instead of master
From: Nicolas de Pesloüan @ 2011-03-10 20:44 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Andy Gospodarek, netdev, davem, shemminger, kaber, fubar,
	eric.dumazet
In-Reply-To: <20110310064829.GA3261@psychotron.redhat.com>

Le 10/03/2011 07:48, Jiri Pirko a écrit :
>> But for all others setups, where there exist some net_devices before
>> the "untagging" one, you would face some troubles. For example, with
>> eth0+eth1 ->  br0 ->  br0.100, you cannot untag before entering
>> __netif_receive_skb. If you do so, the bridge would receive untagged
>> frame and if the frame is not for the local host, the bridge would
>> forward an untagged frame while it is expected to forward a tagged
>> one. Even if the bridge is in a position to know the frame *was*
>> tagged, we cannot expect the bridge to do special processing to
>> handle this situation. Doing so would break layering.
>
> I disagree.
> eth0 ->  untag on early __netif_receive_skb (sets up skb->vlan_tci)
>             ->rx_handler of bridge
> ->br0 ->  tag is detected by vlan_tx_tag_present()
>             ->  reinject to __netif_receive_skb with skb->dev == br0.100

For local delivery, is should work.

But if the bridge must forward the frame to another host (bridges are designed for such things :-)), 
it will have to insert the vlan header back into the frame. I don't understand how it could work 
automagically in this situation.

> This way the flow would be very similar to vlan-hw-accel, am I right?

So your point is to remove any 802.1Q header in any ingres frame, whatever the local interface 
setup. Right?

How would this support nested vlan headers?

eth0 -> eth0.100 -> eth0.100.200 -> eth0.100.200.300.

Who will choose eth0.100.200.300 as the last skb->dev?

> I have following patch in mind. Note it's raw DRAFT.

I need to take some time to review your patch.

In the mean time, what is the status of the whole patch series? Can we expect an ACK from someone?

	Nicolas.

^ permalink raw reply

* Re: Stale entries in RT_TABLE_LOCAL
From: Alex Sidorenko @ 2011-03-10 19:41 UTC (permalink / raw)
  To: Julian Anastasov; +Cc: David Miller, netdev@vger.kernel.org
In-Reply-To: <alpine.LFD.2.00.1103100222080.2550@ja.ssi.bg>

On March 9, 2011 08:50:27 PM Julian Anastasov wrote:
>         Aha, it seems the problem happens when the
> both lines are executed while there is another address on
> device and the last added address becomes secondary
> for the 1st, eg:

Hi Julian,

this is correct - you need another address on device. I used iptest.sh script 
for testing as it is easier and less error-prone than adding addresses 
manually. In additon, it reinserts 'dummy' module so that every time we start 
with a clean slate.

>         Alex, what is the kernel version and can you test
> 
> it on different kernels?

I tested this on SLES10 (2.6.16 kernel), RHEL5 (2.6.18 kernel) and 
Ubuntu/Maverick (2.6.35 kernel)

> 
> IP1: 192.168.140.31/22, primary
> IP2: 192.168.142.109/23, primary
> IP3: 192.168.142.109/22, secondary for primary IP1
> 
>         It is the route for IP3 that is leaked, with prefsrc=IP1.
> We create local route for secondary IPs with prefsrc=ItsPrimaryIP.
> Both local routes for 109 differ in prefsrc (fa_info). But on
> deletion only one route is deleted due to last_ip check - the first
> because on deletion prefsrc is not matched, fib_table_delete
> does not work in symmetric way. So, the local route created
> for IP3 remains no matter the deletion order.
> If we decide to create one unique local route for this case,
> there is a risk device unregistration to remove
> it (fib_sync_down_dev with force > 0). 

Yes, but do we _really_ need to be able to add the same IP multiple times 
(with different masks)? If not, we can just return an error when someine tries 
to add the same IP again with different mask.

Regards,
Alex

-- 
------------------------------------------------------------------
Alexandre Sidorenko             email: asid@hp.com
WTEC Linux			Hewlett-Packard (Canada)
------------------------------------------------------------------

^ permalink raw reply

* [PATCH] conntrack: fix sysctl memory leak
From: Stephen Hemminger @ 2011-03-10 18:14 UTC (permalink / raw)
  To: Patrick McHardy, David S. Miller, netdev; +Cc: netfilter-devel

Message in log because sysctl table was not empty at netns exit
 WARNING: at net/sysctl_net.c:84 sysctl_net_exit+0x2a/0x2c()

Instrumenting showed that the nf_conntrack_timestamp was the entry
that was being created but not cleared.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

--- a/net/netfilter/nf_conntrack_core.c	2011-03-10 09:42:10.656565623 -0800
+++ b/net/netfilter/nf_conntrack_core.c	2011-03-10 09:42:30.652821904 -0800
@@ -1301,6 +1301,7 @@ static void nf_conntrack_cleanup_net(str
 
 	nf_ct_free_hashtable(net->ct.hash, net->ct.htable_size);
 	nf_conntrack_ecache_fini(net);
+	nf_conntrack_tstamp_fini(net);
 	nf_conntrack_acct_fini(net);
 	nf_conntrack_expect_fini(net);
 	kmem_cache_destroy(net->ct.nf_conntrack_cachep);

^ permalink raw reply

* [PATCH] ipv6: netfilter: ip6_tables: fix infoleak to userspace
From: Vasiliy Kulikov @ 2011-03-10 18:13 UTC (permalink / raw)
  To: linux-kernel
  Cc: security, Patrick McHardy, David S. Miller, Alexey Kuznetsov,
	James Morris, Hideaki YOSHIFUJI, netfilter-devel, netfilter,
	coreteam, netdev

Structures ip6t_replace, compat_ip6t_replace, and xt_get_revision are
copied from userspace.  Fields of these structs that are
zero-terminated strings are not checked.  When they are used as argument
to a format string containing "%s" in request_module(), some sensitive
information is leaked to userspace via argument of spawned modprobe
process.

The first bug was introduced before the git epoch;  the second was
introduced in 3bc3fe5e (v2.6.25-rc1);  the third is introduced by
6b7d31fc (v2.6.15-rc1).  To trigger the bug one should have
CAP_NET_ADMIN.

Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
 Compile tested.

 net/ipv6/netfilter/ip6_tables.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index 47b7b8d..c9598a9 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -1275,6 +1275,7 @@ do_replace(struct net *net, const void __user *user, unsigned int len)
 	/* overflow check */
 	if (tmp.num_counters >= INT_MAX / sizeof(struct xt_counters))
 		return -ENOMEM;
+	tmp.name[sizeof(tmp.name)-1] = 0;
 
 	newinfo = xt_alloc_table_info(tmp.size);
 	if (!newinfo)
@@ -1822,6 +1823,7 @@ compat_do_replace(struct net *net, void __user *user, unsigned int len)
 		return -ENOMEM;
 	if (tmp.num_counters >= INT_MAX / sizeof(struct xt_counters))
 		return -ENOMEM;
+	tmp.name[sizeof(tmp.name)-1] = 0;
 
 	newinfo = xt_alloc_table_info(tmp.size);
 	if (!newinfo)
@@ -2051,6 +2053,7 @@ do_ip6t_get_ctl(struct sock *sk, int cmd, void __user *user, int *len)
 			ret = -EFAULT;
 			break;
 		}
+		rev.name[sizeof(rev.name)-1] = 0;
 
 		if (cmd == IP6T_SO_GET_REVISION_TARGET)
 			target = 1;
-- 
1.7.0.4

^ permalink raw reply related

* [PATCH] ipv4: netfilter: ipt_CLUSTERIP: fix buffer overflow
From: Vasiliy Kulikov @ 2011-03-10 18:12 UTC (permalink / raw)
  To: linux-kernel
  Cc: security, Patrick McHardy, David S. Miller, Alexey Kuznetsov,
	James Morris, Hideaki YOSHIFUJI, netfilter-devel, netfilter,
	coreteam, netdev

buffer string is copied from userspace.  It is not checked whether it is
zero terminated.  This may lead to overflow inside of simple_strtoul().

It was introduced before the git epoch.  Files "ipt_CLUSTERIP/*" are
root writable only by default, however, on some setups permissions might be
relaxed to e.g. network admin user.

Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
 Compile tested.

 net/ipv4/netfilter/ipt_CLUSTERIP.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/net/ipv4/netfilter/ipt_CLUSTERIP.c b/net/ipv4/netfilter/ipt_CLUSTERIP.c
index 403ca57..7aabf9a 100644
--- a/net/ipv4/netfilter/ipt_CLUSTERIP.c
+++ b/net/ipv4/netfilter/ipt_CLUSTERIP.c
@@ -666,6 +666,7 @@ static ssize_t clusterip_proc_write(struct file *file, const char __user *input,
 
 	if (copy_from_user(buffer, input, PROC_WRITELEN))
 		return -EFAULT;
+	buffer[sizeof(buffer)-1] = 0;
 
 	if (*buffer == '+') {
 		nodenum = simple_strtoul(buffer+1, NULL, 10);
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH] ipv4: netfilter: ip_tables: fix infoleak to userspace
From: Vasiliy Kulikov @ 2011-03-10 18:12 UTC (permalink / raw)
  To: linux-kernel
  Cc: security, Patrick McHardy, David S. Miller, Alexey Kuznetsov,
	James Morris, Hideaki YOSHIFUJI, netfilter-devel, netfilter,
	coreteam, netdev

Structures ipt_replace, compat_ipt_replace, and xt_get_revision are
copied from userspace.  Fields of these structs that are
zero-terminated strings are not checked.  When they are used as argument
to a format string containing "%s" in request_module(), some sensitive
information is leaked to userspace via argument of spawned modprobe
process.

The first and the third bugs were introduced before the git epoch; the
second was introduced in 2722971c (v2.6.17-rc1).  To trigger the bug
one should have CAP_NET_ADMIN.

Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
 Compile tested.

 net/ipv4/netfilter/ip_tables.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index ef7d7b9..b09ed0d 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -1262,6 +1262,7 @@ do_replace(struct net *net, const void __user *user, unsigned int len)
 	/* overflow check */
 	if (tmp.num_counters >= INT_MAX / sizeof(struct xt_counters))
 		return -ENOMEM;
+	tmp.name[sizeof(tmp.name)-1] = 0;
 
 	newinfo = xt_alloc_table_info(tmp.size);
 	if (!newinfo)
@@ -1807,6 +1808,7 @@ compat_do_replace(struct net *net, void __user *user, unsigned int len)
 		return -ENOMEM;
 	if (tmp.num_counters >= INT_MAX / sizeof(struct xt_counters))
 		return -ENOMEM;
+	tmp.name[sizeof(tmp.name)-1] = 0;
 
 	newinfo = xt_alloc_table_info(tmp.size);
 	if (!newinfo)
@@ -2036,6 +2038,7 @@ do_ipt_get_ctl(struct sock *sk, int cmd, void __user *user, int *len)
 			ret = -EFAULT;
 			break;
 		}
+		rev.name[sizeof(rev.name)-1] = 0;
 
 		if (cmd == IPT_SO_GET_REVISION_TARGET)
 			target = 1;
-- 
1.7.0.4

^ permalink raw reply related

* [PATCH] ipv4: netfilter: arp_tables: fix infoleak to userspace
From: Vasiliy Kulikov @ 2011-03-10 18:12 UTC (permalink / raw)
  To: linux-kernel
  Cc: security, Patrick McHardy, David S. Miller, Alexey Kuznetsov,
	James Morris, Hideaki YOSHIFUJI, netfilter-devel, netfilter,
	coreteam, netdev

Structures ipt_replace, compat_ipt_replace, and xt_get_revision are
copied from userspace.  Fields of these structs that are
zero-terminated strings are not checked.  When they are used as argument
to a format string containing "%s" in request_module(), some sensitive
information is leaked to userspace via argument of spawned modprobe
process.

The first bug was introduced before the git epoch;  the second is
introduced by 6b7d31fc (v2.6.15-rc1);  the third is introduced by
6b7d31fc (v2.6.15-rc1).  To trigger the bug one should have
CAP_NET_ADMIN.

Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
 Compile tested.

 net/ipv4/netfilter/arp_tables.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index e95054c..4b5d457 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -1066,6 +1066,7 @@ static int do_replace(struct net *net, const void __user *user,
 	/* overflow check */
 	if (tmp.num_counters >= INT_MAX / sizeof(struct xt_counters))
 		return -ENOMEM;
+	tmp.name[sizeof(tmp.name)-1] = 0;
 
 	newinfo = xt_alloc_table_info(tmp.size);
 	if (!newinfo)
@@ -1488,6 +1489,7 @@ static int compat_do_replace(struct net *net, void __user *user,
 		return -ENOMEM;
 	if (tmp.num_counters >= INT_MAX / sizeof(struct xt_counters))
 		return -ENOMEM;
+	tmp.name[sizeof(tmp.name)-1] = 0;
 
 	newinfo = xt_alloc_table_info(tmp.size);
 	if (!newinfo)
@@ -1740,6 +1742,7 @@ static int do_arpt_get_ctl(struct sock *sk, int cmd, void __user *user, int *len
 			ret = -EFAULT;
 			break;
 		}
+		rev.name[sizeof(rev.name)-1] = 0;
 
 		try_then_request_module(xt_find_revision(NFPROTO_ARP, rev.name,
 							 rev.revision, 1, &ret),
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH] net: convert xen-netfront to hw_features, [PoC PATCH] net: convert xen-netfront to hw_features
From: Michał Mirosław @ 2011-03-10 17:47 UTC (permalink / raw)
  To: Ian Campbell
  Cc: netdev@vger.kernel.org, xen-devel, Ben Hutchings,
	Jeremy Fitzhardinge, Herbert Xu, Konrad Rzeszutek Wilk,
	Francois Romieu
In-Reply-To: <AANLkTinHyFLRkeb60x4Q6uUw6p43XBjk5rpqCMo49WKF@mail.gmail.com>

[resent because of broken headers]

This patch converts xen-netfront to hw_features.

It's not tested in any way. The original code for offload setting seems
broken as it resets the features on every netback reconnect.

This will set GSO_ROBUST at device creation time (earlier than connect time).

RX checksum offload is forced on - so advertise as it is.

netdev_update_features() should be called whenever offload conditions change.
This in turn calls ndo_fix_features() and ndo_set_features() to verify them.
I leave this to you as I don't know when or how the notifications get passed.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/net/xen-netfront.c |   57 +++++++++++++++++--------------------------
 1 files changed, 23 insertions(+), 34 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index da1f121..c642fd3 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -1148,6 +1148,8 @@ static const struct net_device_ops xennet_netdev_ops = {
 	.ndo_change_mtu	     = xennet_change_mtu,
 	.ndo_set_mac_address = eth_mac_addr,
 	.ndo_validate_addr   = eth_validate_addr,
+	.ndo_fix_features    = xennet_fix_features,
+	.ndo_set_features    = xennet_set_features,
 };
 
 static struct net_device * __devinit xennet_create_dev(struct xenbus_device *dev)
@@ -1209,7 +1211,9 @@ static struct net_device * __devinit xennet_create_dev(struct xenbus_device *dev
 	netdev->netdev_ops	= &xennet_netdev_ops;
 
 	netif_napi_add(netdev, &np->napi, xennet_poll, 64);
-	netdev->features        = NETIF_F_IP_CSUM;
+	netdev->features        = NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
+				  NETIF_F_GSO_ROBUST;
+	netdev->hw_features	= NETIF_F_IP_CSUM | NETIF_F_SG | NETIF_F_TSO;
 
 	SET_ETHTOOL_OPS(netdev, &xennet_ethtool_ops);
 	SET_NETDEV_DEV(netdev, &dev->dev);
@@ -1510,52 +1514,40 @@ again:
 	return err;
 }
 
-static int xennet_set_sg(struct net_device *dev, u32 data)
+static u32 xennet_fix_features(struct net_device *dev, u32 features)
 {
-	if (data) {
-		struct netfront_info *np = netdev_priv(dev);
-		int val;
+	struct netfront_info *np = netdev_priv(dev);
+	int val;
 
+	if (features & NETIF_F_SG) {
 		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend, "feature-sg",
 				 "%d", &val) < 0)
 			val = 0;
+
 		if (!val)
-			return -ENOSYS;
-	} else if (dev->mtu > ETH_DATA_LEN)
-		dev->mtu = ETH_DATA_LEN;
-
-	return ethtool_op_set_sg(dev, data);
-}
-
-static int xennet_set_tso(struct net_device *dev, u32 data)
-{
-	if (data) {
-		struct netfront_info *np = netdev_priv(dev);
-		int val;
+			features &= ~NETIF_F_SG;
+	}
 
+	if (features & NETIF_F_TSO) {
 		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend,
 				 "feature-gso-tcpv4", "%d", &val) < 0)
 			val = 0;
+
 		if (!val)
-			return -ENOSYS;
+			features &= ~NETIF_F_TSO;
 	}
 
-	return ethtool_op_set_tso(dev, data);
+	return features;
 }
 
-static void xennet_set_features(struct net_device *dev)
+static int xennet_set_features(struct net_device *dev, u32 features)
 {
-	/* Turn off all GSO bits except ROBUST. */
-	dev->features &= ~NETIF_F_GSO_MASK;
-	dev->features |= NETIF_F_GSO_ROBUST;
-	xennet_set_sg(dev, 0);
+	if (!(features & NETIF_F_SG) && dev->mtu > ETH_DATA_LEN) {
+		netdev_info(dev, "Reducing MTU because no SG offload");
+		dev->mtu = ETH_DATA_LEN;
+	}
 
-	/* We need checksum offload to enable scatter/gather and TSO. */
-	if (!(dev->features & NETIF_F_IP_CSUM))
-		return;
-
-	if (!xennet_set_sg(dev, 1))
-		xennet_set_tso(dev, 1);
+	return 0;
 }
 
 static int xennet_connect(struct net_device *dev)
@@ -1582,7 +1574,7 @@ static int xennet_connect(struct net_device *dev)
 	if (err)
 		return err;
 
-	xennet_set_features(dev);
+	netdev_update_features(dev);
 
 	spin_lock_bh(&np->rx_lock);
 	spin_lock_irq(&np->tx_lock);
@@ -1710,9 +1702,6 @@ static void xennet_get_strings(struct net_device *dev, u32 stringset, u8 * data)
 
 static const struct ethtool_ops xennet_ethtool_ops =
 {
-	.set_tx_csum = ethtool_op_set_tx_csum,
-	.set_sg = xennet_set_sg,
-	.set_tso = xennet_set_tso,
 	.get_link = ethtool_op_get_link,
 
 	.get_sset_count = xennet_get_sset_count,
-- 
1.7.2.3


^ permalink raw reply related

* [PoC PATCH] net: convert xen-netfront to hw_features
From: Michał Mirosław, Michał Mirosław @ 2011-03-10 17:43 UTC (permalink / raw)
  To: Ian Campbell
  Cc: Jeremy Fitzhardinge, xen-devel, Herbert Xu, Konrad Rzeszutek Wilk,
	netdev@vger.kernel.org, Francois Romieu, Ben Hutchings
In-Reply-To: <AANLkTinHyFLRkeb60x4Q6uUw6p43XBjk5rpqCMo49WKF@mail.gmail.com>

This patch converts xen-netfront to hw_features.

It's not tested in any way. The original code for offload setting seems
broken as it resets the features on every netback reconnect.

This will set GSO_ROBUST at device creation time (earlier than connect time).

RX checksum offload is forced on - so advertise as it is.

netdev_update_features() should be called whenever offload conditions change.
This in turn calls ndo_fix_features() and ndo_set_features() to verify them.
I leave this to you as I don't know when or how the notifications get passed.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/net/xen-netfront.c |   57 +++++++++++++++++--------------------------
 1 files changed, 23 insertions(+), 34 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index da1f121..c642fd3 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -1148,6 +1148,8 @@ static const struct net_device_ops xennet_netdev_ops = {
 	.ndo_change_mtu	     = xennet_change_mtu,
 	.ndo_set_mac_address = eth_mac_addr,
 	.ndo_validate_addr   = eth_validate_addr,
+	.ndo_fix_features    = xennet_fix_features,
+	.ndo_set_features    = xennet_set_features,
 };
 
 static struct net_device * __devinit xennet_create_dev(struct xenbus_device *dev)
@@ -1209,7 +1211,9 @@ static struct net_device * __devinit xennet_create_dev(struct xenbus_device *dev
 	netdev->netdev_ops	= &xennet_netdev_ops;
 
 	netif_napi_add(netdev, &np->napi, xennet_poll, 64);
-	netdev->features        = NETIF_F_IP_CSUM;
+	netdev->features        = NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
+				  NETIF_F_GSO_ROBUST;
+	netdev->hw_features	= NETIF_F_IP_CSUM | NETIF_F_SG | NETIF_F_TSO;
 
 	SET_ETHTOOL_OPS(netdev, &xennet_ethtool_ops);
 	SET_NETDEV_DEV(netdev, &dev->dev);
@@ -1510,52 +1514,40 @@ again:
 	return err;
 }
 
-static int xennet_set_sg(struct net_device *dev, u32 data)
+static u32 xennet_fix_features(struct net_device *dev, u32 features)
 {
-	if (data) {
-		struct netfront_info *np = netdev_priv(dev);
-		int val;
+	struct netfront_info *np = netdev_priv(dev);
+	int val;
 
+	if (features & NETIF_F_SG) {
 		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend, "feature-sg",
 				 "%d", &val) < 0)
 			val = 0;
+
 		if (!val)
-			return -ENOSYS;
-	} else if (dev->mtu > ETH_DATA_LEN)
-		dev->mtu = ETH_DATA_LEN;
-
-	return ethtool_op_set_sg(dev, data);
-}
-
-static int xennet_set_tso(struct net_device *dev, u32 data)
-{
-	if (data) {
-		struct netfront_info *np = netdev_priv(dev);
-		int val;
+			features &= ~NETIF_F_SG;
+	}
 
+	if (features & NETIF_F_TSO) {
 		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend,
 				 "feature-gso-tcpv4", "%d", &val) < 0)
 			val = 0;
+
 		if (!val)
-			return -ENOSYS;
+			features &= ~NETIF_F_TSO;
 	}
 
-	return ethtool_op_set_tso(dev, data);
+	return features;
 }
 
-static void xennet_set_features(struct net_device *dev)
+static int xennet_set_features(struct net_device *dev, u32 features)
 {
-	/* Turn off all GSO bits except ROBUST. */
-	dev->features &= ~NETIF_F_GSO_MASK;
-	dev->features |= NETIF_F_GSO_ROBUST;
-	xennet_set_sg(dev, 0);
+	if (!(features & NETIF_F_SG) && dev->mtu > ETH_DATA_LEN) {
+		netdev_info(dev, "Reducing MTU because no SG offload");
+		dev->mtu = ETH_DATA_LEN;
+	}
 
-	/* We need checksum offload to enable scatter/gather and TSO. */
-	if (!(dev->features & NETIF_F_IP_CSUM))
-		return;
-
-	if (!xennet_set_sg(dev, 1))
-		xennet_set_tso(dev, 1);
+	return 0;
 }
 
 static int xennet_connect(struct net_device *dev)
@@ -1582,7 +1574,7 @@ static int xennet_connect(struct net_device *dev)
 	if (err)
 		return err;
 
-	xennet_set_features(dev);
+	netdev_update_features(dev);
 
 	spin_lock_bh(&np->rx_lock);
 	spin_lock_irq(&np->tx_lock);
@@ -1710,9 +1702,6 @@ static void xennet_get_strings(struct net_device *dev, u32 stringset, u8 * data)
 
 static const struct ethtool_ops xennet_ethtool_ops =
 {
-	.set_tx_csum = ethtool_op_set_tx_csum,
-	.set_sg = xennet_set_sg,
-	.set_tso = xennet_set_tso,
 	.get_link = ethtool_op_get_link,
 
 	.get_sset_count = xennet_get_sset_count,
-- 
1.7.2.3

^ permalink raw reply related

* Re: [GIT/PATCH v4] xen network backend driver
From: Michał Mirosław @ 2011-03-10 17:33 UTC (permalink / raw)
  To: Ian Campbell
  Cc: netdev@vger.kernel.org, xen-devel, Ben Hutchings,
	Jeremy Fitzhardinge, Herbert Xu, Konrad Rzeszutek Wilk,
	Francois Romieu, Michał Mirosław
In-Reply-To: <1299776554.17339.824.camel@zakaz.uk.xensource.com>

2011/3/10 Ian Campbell <Ian.Campbell@citrix.com>:
> The following patch is the fourth iteration of the Xen network backend
> driver for upstream Linux.
>
> This driver ("netback") is the host side counterpart to the frontend
> driver in drivers/net/xen-netfront.c. The PV protocol is also
> implemented by frontend drivers in other OSes too, such as the BSDs and
> even Windows.
[...]
> --- /dev/null
> +++ b/drivers/net/xen-netback/common.h
> @@ -0,0 +1,162 @@
[...]
> +struct xenvif {
[...]
> +       /* Flags that must not be set in dev->features */
> +       int features_disabled;
> +
> +       /* Frontend feature information. */
> +       u8 can_sg:1;
> +       u8 gso:1;
> +       u8 gso_prefix:1;
> +       u8 csum:1;
[...]
> --- /dev/null
> +++ b/drivers/net/xen-netback/interface.c
> @@ -0,0 +1,424 @@
[...]
> +static int xenvif_change_mtu(struct net_device *dev, int mtu)
> +{
> +       struct xenvif *vif = netdev_priv(dev);
> +       int max = vif->can_sg ? 65535 - VLAN_ETH_HLEN : ETH_DATA_LEN;
> +
> +       if (mtu > max)
> +               return -EINVAL;
> +       dev->mtu = mtu;
> +       return 0;
> +}
> +
> +static void xenvif_set_features(struct xenvif *vif)
> +{
> +       struct net_device *dev = vif->dev;
> +       int features = dev->features;
> +
> +       if (vif->can_sg)
> +               features |= NETIF_F_SG;
> +       if (vif->gso || vif->gso_prefix)
> +               features |= NETIF_F_TSO;
> +       if (vif->csum)
> +               features |= NETIF_F_IP_CSUM;
> +
> +       features &= ~(vif->features_disabled);
> +
> +       if (!(features & NETIF_F_SG) && dev->mtu > ETH_DATA_LEN)
> +               dev->mtu = ETH_DATA_LEN;
> +
> +       dev->features = features;
> +}
> +
> +static int xenvif_set_tx_csum(struct net_device *dev, u32 data)
> +{
> +       struct xenvif *vif = netdev_priv(dev);
> +       if (data) {
> +               if (!vif->csum)
> +                       return -EOPNOTSUPP;
> +               vif->features_disabled &= ~NETIF_F_IP_CSUM;
> +       } else {
> +               vif->features_disabled |= NETIF_F_IP_CSUM;
> +       }
> +
> +       xenvif_set_features(vif);
> +       return 0;
> +}
> +
> +static int xenvif_set_sg(struct net_device *dev, u32 data)
> +{
> +       struct xenvif *vif = netdev_priv(dev);
> +       if (data) {
> +               if (!vif->can_sg)
> +                       return -EOPNOTSUPP;
> +               vif->features_disabled &= ~NETIF_F_SG;
> +       } else {
> +               vif->features_disabled |= NETIF_F_SG;
> +       }
> +
> +       xenvif_set_features(vif);
> +       return 0;
> +}
> +
> +static int xenvif_set_tso(struct net_device *dev, u32 data)
> +{
> +       struct xenvif *vif = netdev_priv(dev);
> +       if (data) {
> +               if (!vif->gso && !vif->gso_prefix)
> +                       return -EOPNOTSUPP;
> +               vif->features_disabled &= ~NETIF_F_TSO;
> +       } else {
> +               vif->features_disabled |= NETIF_F_TSO;
> +       }
> +
> +       xenvif_set_features(vif);
> +       return 0;
> +}
[...]

You could make this simpler by using netdev->hw_features and
ndo_fix_features/ndo_set_features calls recently introduced in
net-next. I'll reply with a proof-of-concept patch for xen-netfront.

Best Regards,
Michał Mirosław

^ permalink raw reply

* Re: [GIT/PATCH v4] xen network backend driver
From: Ben Hutchings @ 2011-03-10 17:30 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Ian Campbell, netdev@vger.kernel.org, xen-devel,
	Jeremy Fitzhardinge, Herbert Xu, Konrad Rzeszutek Wilk,
	Francois Romieu
In-Reply-To: <20110310091555.0fa8764d@nehalam>

On Thu, 2011-03-10 at 09:15 -0800, Stephen Hemminger wrote:
> On Thu, 10 Mar 2011 17:02:33 +0000
> Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > +struct xenvif {
> > +	/* Unique identifier for this interface. */
> ...
> > +	struct net_device_stats stats;
> > +
> 
> There is already a stats struct in net_device in current kernel
> versions, unless there is a compelling reason otherwise
> please use that.
> 
> Also, you probably want to implement per-cpu and 64 bit
> stats.

The driver is using a single queue, so I don't see what benefit it would
get from per-cpu stats.  At some point it should become multiqueue and
then it should store per-queue stats.

64-bit stats are definitely preferable, but since they're being
maintained on the data path this may require some significant work.
(Ian: see <linux/u64_stats_sync.h> for the canonical way to do this.)
Given that only a relatively few existing drivers do this (I count 13),
I'm not sure we can reasonably demand that a new driver does - yet.

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

* [patch]iwch_cm: fix possible null pointer
From: j223yang @ 2011-03-10 17:23 UTC (permalink / raw)
  To: davem, netdev; +Cc: linux-kernel

This patch fixes bugzilla #9583:
https://bugzilla.kernel.org/show_bug.cgi?id=9583

The patch adds NULL check for 'ep->com.cm_id' before derefenrence, since
'ep->com.cm_id' is testing for NULL before. But it depends on the relationship
between 'status' and 'ep->com.cm_id'.

Signed-off-by: Jinqiu Yang<crindy646@gmail.com>
---
 iwch_cm.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--- a/drivers/infiniband/hw/cxgb3/iwch_cm.c	2011-03-10 11:20:13.641039456 -0500
+++ b/drivers/infiniband/hw/cxgb3/iwch_cm.c	2011-03-10 11:39:03.346164807 -0500
@@ -746,7 +746,8 @@ static void connect_reply_upcall(struct 
 		ep->com.cm_id->event_handler(ep->com.cm_id, &event);
 	}
 	if (status < 0) {
-		ep->com.cm_id->rem_ref(ep->com.cm_id);
+		if (ep->com.cm_id)
+			ep->com.cm_id->rem_ref(ep->com.cm_id);
 		ep->com.cm_id = NULL;
 		ep->com.qp = NULL;
 	}

^ permalink raw reply

* [Patch]ariadne: remove redundant NULL check
From: j223yang @ 2011-03-10 17:20 UTC (permalink / raw)
  To: davem, netdev; +Cc: linux-kernel

Simply remove redundant 'dev' NULL check. 

Signed-off-by: Jinqiu Yang<crindy646@gmail.com>
---
 ariadne.c |    5 -----
 1 file changed, 5 deletions(-)

--- a/drivers/net/ariadne.c	2011-03-10 11:20:14.076039323 -0500
+++ b/drivers/net/ariadne.c	2011-03-10 11:21:59.111164537 -0500
@@ -425,11 +425,6 @@ static irqreturn_t ariadne_interrupt(int
     int csr0, boguscnt;
     int handled = 0;
 
-    if (dev == NULL) {
-	printk(KERN_WARNING "ariadne_interrupt(): irq for unknown device.\n");
-	return IRQ_NONE;
-    }
-
     lance->RAP = CSR0;			/* PCnet-ISA Controller Status */
 
     if (!(lance->RDP & INTR))		/* Check if any interrupt has been */

^ 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