Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 2/2 net-next-2.6] vlan: Precise RX stats accounting
From: Eric Dumazet @ 2009-11-17 14:53 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20091117.002037.229474882.davem@davemloft.net>

David Miller a écrit :
> 
> Eric, please make allocation failure cause the vlan_setup()
> to fail and propagate -EFAULT back to the caller.
> 

Given that vlan_setup(struct net_device *) is void and cannot
return error status, unless using a global variable (ugly),
or changing all setup() prototypes (oh well...)

One possibility is to perform the allocation in vlan_dev_init()
(called from register_netdevice()), and freeing it in vlan_dev_uninit().

Thanks

[PATCH net-next-2.6 take2] vlan: Precise RX stats accounting

With multi queue devices, its possible that several cpus call
vlan RX routines simultaneously for the same vlan device.

We update RX stats counter without any locking, so we can
get slightly wrong counters.

One possible fix is to use percpu counters, to get precise
accounting and also get guarantee of no cache line ping pongs
between cpus.

Note: this adds 16 bytes (32 bytes on 64bit arches) of percpu
data per vlan device.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 net/8021q/vlan.h      |   17 ++++++++++++++
 net/8021q/vlan_core.c |   12 +++++-----
 net/8021q/vlan_dev.c  |   47 ++++++++++++++++++++++++++++++++++------
 3 files changed, 65 insertions(+), 11 deletions(-)

diff --git a/net/8021q/vlan.h b/net/8021q/vlan.h
index 68f9290..5685296 100644
--- a/net/8021q/vlan.h
+++ b/net/8021q/vlan.h
@@ -16,6 +16,21 @@ struct vlan_priority_tci_mapping {
 	struct vlan_priority_tci_mapping	*next;
 };
 
+
+/**
+ *	struct vlan_rx_stats - VLAN percpu rx stats
+ *	@rx_packets: number of received packets
+ *	@rx_bytes: number of received bytes
+ *	@multicast: number of received multicast packets
+ *	@rx_errors: number of errors
+ */
+struct vlan_rx_stats {
+	unsigned long rx_packets;
+	unsigned long rx_bytes;
+	unsigned long multicast;
+	unsigned long rx_errors;
+};
+
 /**
  *	struct vlan_dev_info - VLAN private device data
  *	@nr_ingress_mappings: number of ingress priority mappings
@@ -29,6 +44,7 @@ struct vlan_priority_tci_mapping {
  *	@dent: proc dir entry
  *	@cnt_inc_headroom_on_tx: statistic - number of skb expansions on TX
  *	@cnt_encap_on_xmit: statistic - number of skb encapsulations on TX
+ *	@vlan_rx_stats: ptr to percpu rx stats
  */
 struct vlan_dev_info {
 	unsigned int				nr_ingress_mappings;
@@ -45,6 +61,7 @@ struct vlan_dev_info {
 	struct proc_dir_entry			*dent;
 	unsigned long				cnt_inc_headroom_on_tx;
 	unsigned long				cnt_encap_on_xmit;
+	struct vlan_rx_stats			*vlan_rx_stats;
 };
 
 static inline struct vlan_dev_info *vlan_dev_info(const struct net_device *dev)
diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index 971d375..e75a2f3 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -31,7 +31,7 @@ EXPORT_SYMBOL(__vlan_hwaccel_rx);
 int vlan_hwaccel_do_receive(struct sk_buff *skb)
 {
 	struct net_device *dev = skb->dev;
-	struct net_device_stats *stats;
+	struct vlan_rx_stats     *rx_stats;
 
 	skb->dev = vlan_dev_info(dev)->real_dev;
 	netif_nit_deliver(skb);
@@ -40,15 +40,17 @@ int vlan_hwaccel_do_receive(struct sk_buff *skb)
 	skb->priority = vlan_get_ingress_priority(dev, skb->vlan_tci);
 	skb->vlan_tci = 0;
 
-	stats = &dev->stats;
-	stats->rx_packets++;
-	stats->rx_bytes += skb->len;
+	rx_stats = per_cpu_ptr(vlan_dev_info(dev)->vlan_rx_stats,
+			       smp_processor_id());
+
+	rx_stats->rx_packets++;
+	rx_stats->rx_bytes += skb->len;
 
 	switch (skb->pkt_type) {
 	case PACKET_BROADCAST:
 		break;
 	case PACKET_MULTICAST:
-		stats->multicast++;
+		rx_stats->multicast++;
 		break;
 	case PACKET_OTHERHOST:
 		/* Our lower layer thinks this is not local, let's make sure.
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 9159659..de0dc6b 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -140,7 +140,7 @@ int vlan_skb_recv(struct sk_buff *skb, struct net_device *dev,
 		  struct packet_type *ptype, struct net_device *orig_dev)
 {
 	struct vlan_hdr *vhdr;
-	struct net_device_stats *stats;
+	struct vlan_rx_stats *rx_stats;
 	u16 vlan_id;
 	u16 vlan_tci;
 
@@ -163,9 +163,10 @@ int vlan_skb_recv(struct sk_buff *skb, struct net_device *dev,
 		goto err_unlock;
 	}
 
-	stats = &skb->dev->stats;
-	stats->rx_packets++;
-	stats->rx_bytes += skb->len;
+	rx_stats = per_cpu_ptr(vlan_dev_info(dev)->vlan_rx_stats,
+			       smp_processor_id());
+	rx_stats->rx_packets++;
+	rx_stats->rx_bytes += skb->len;
 
 	skb_pull_rcsum(skb, VLAN_HLEN);
 
@@ -180,7 +181,7 @@ int vlan_skb_recv(struct sk_buff *skb, struct net_device *dev,
 		break;
 
 	case PACKET_MULTICAST:
-		stats->multicast++;
+		rx_stats->multicast++;
 		break;
 
 	case PACKET_OTHERHOST:
@@ -200,7 +201,7 @@ int vlan_skb_recv(struct sk_buff *skb, struct net_device *dev,
 
 	skb = vlan_check_reorder_header(skb);
 	if (!skb) {
-		stats->rx_errors++;
+		rx_stats->rx_errors++;
 		goto err_unlock;
 	}
 
@@ -731,6 +732,11 @@ static int vlan_dev_init(struct net_device *dev)
 		subclass = 1;
 
 	vlan_dev_set_lockdep_class(dev, subclass);
+
+	vlan_dev_info(dev)->vlan_rx_stats = alloc_percpu(struct vlan_rx_stats);
+	if (!vlan_dev_info(dev)->vlan_rx_stats)
+		return -ENOMEM;
+
 	return 0;
 }
 
@@ -740,6 +746,8 @@ static void vlan_dev_uninit(struct net_device *dev)
 	struct vlan_dev_info *vlan = vlan_dev_info(dev);
 	int i;
 
+	free_percpu(vlan->vlan_rx_stats);
+	vlan->vlan_rx_stats = NULL;
 	for (i = 0; i < ARRAY_SIZE(vlan->egress_priority_map); i++) {
 		while ((pm = vlan->egress_priority_map[i]) != NULL) {
 			vlan->egress_priority_map[i] = pm->next;
@@ -775,6 +783,31 @@ static u32 vlan_ethtool_get_flags(struct net_device *dev)
 	return dev_ethtool_get_flags(vlan->real_dev);
 }
 
+static struct net_device_stats *vlan_dev_get_stats(struct net_device *dev)
+{
+	struct net_device_stats *stats = &dev->stats;
+
+	dev_txq_stats_fold(dev, stats);
+
+	if (vlan_dev_info(dev)->vlan_rx_stats) {
+		struct vlan_rx_stats *p, rx = {0};
+		int i;
+
+		for_each_possible_cpu(i) {
+			p = per_cpu_ptr(vlan_dev_info(dev)->vlan_rx_stats, i);
+			rx.rx_packets += p->rx_packets;
+			rx.rx_bytes   += p->rx_bytes;
+			rx.rx_errors  += p->rx_errors;
+			rx.multicast  += p->multicast;
+		}
+		stats->rx_packets = rx.rx_packets;
+		stats->rx_bytes   = rx.rx_bytes;
+		stats->rx_errors  = rx.rx_errors;
+		stats->multicast  = rx.multicast;
+	}
+	return stats;
+}
+
 static const struct ethtool_ops vlan_ethtool_ops = {
 	.get_settings	        = vlan_ethtool_get_settings,
 	.get_drvinfo	        = vlan_ethtool_get_drvinfo,
@@ -797,6 +830,7 @@ static const struct net_device_ops vlan_netdev_ops = {
 	.ndo_change_rx_flags	= vlan_dev_change_rx_flags,
 	.ndo_do_ioctl		= vlan_dev_ioctl,
 	.ndo_neigh_setup	= vlan_dev_neigh_setup,
+	.ndo_get_stats		= vlan_dev_get_stats,
 #if defined(CONFIG_FCOE) || defined(CONFIG_FCOE_MODULE)
 	.ndo_fcoe_ddp_setup	= vlan_dev_fcoe_ddp_setup,
 	.ndo_fcoe_ddp_done	= vlan_dev_fcoe_ddp_done,
@@ -820,6 +854,7 @@ static const struct net_device_ops vlan_netdev_accel_ops = {
 	.ndo_change_rx_flags	= vlan_dev_change_rx_flags,
 	.ndo_do_ioctl		= vlan_dev_ioctl,
 	.ndo_neigh_setup	= vlan_dev_neigh_setup,
+	.ndo_get_stats		= vlan_dev_get_stats,
 #if defined(CONFIG_FCOE) || defined(CONFIG_FCOE_MODULE)
 	.ndo_fcoe_ddp_setup	= vlan_dev_fcoe_ddp_setup,
 	.ndo_fcoe_ddp_done	= vlan_dev_fcoe_ddp_done,

^ permalink raw reply related

* Re: [PATCH] net/s390 drivers: add missing 'const' attribute
From: David Miller @ 2009-11-17 14:47 UTC (permalink / raw)
  To: heiko.carstens; +Cc: braunu, netdev
In-Reply-To: <20091117134815.GE5124@osiris.boeblingen.de.ibm.com>

From: Heiko Carstens <heiko.carstens@de.ibm.com>
Date: Tue, 17 Nov 2009 14:48:15 +0100

> Add missing 'const' attribute to avoid the following compile warnings:
> 
> drivers/s390/net/ctcm_main.c: In function 'ctcm_init':
> drivers/s390/net/ctcm_main.c:1864: warning: assignment from incompatible pointer type
> drivers/s390/net/lcs.c: In function 'lcs_init_module':
> drivers/s390/net/lcs.c:2468: warning: assignment from incompatible pointer type
> drivers/s390/net/claw.c: In function 'claw_init':
> drivers/s390/net/claw.c:3408: warning: assignment from incompatible pointer type
> 
> Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>

Applied to net-next-2.6, thank you.

^ permalink raw reply

* Re: [PATCH] vlan: Fix register_vlan_dev() error path
From: David Miller @ 2009-11-17 14:45 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev, kaber
In-Reply-To: <4B02B616.3020801@gmail.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 17 Nov 2009 15:41:26 +0100

> David Miller a écrit :
> 
>> Eric, please make allocation failure cause the vlan_setup()
>> to fail and propagate -EFAULT back to the caller.
> 
> Sure ! (You meant -ENOMEM probably...)

Indeed.

> [PATCH] vlan: Fix register_vlan_dev() error path
> 
> In case register_netdevice() returns an error, and a new vlan_group was
> allocated and inserted in vlan_group_hash[] we call vlan_group_free() without
> deleting group from hash table. Future lookups can give infinite loops or crashes.
> 
> We must delete the vlan_group using RCU safe procedure.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Good catch, applied to net-2.6 and queued up for -stable.

^ permalink raw reply

* [PATCH] vlan: Fix register_vlan_dev() error path
From: Eric Dumazet @ 2009-11-17 14:41 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Patrick McHardy
In-Reply-To: <20091117.002037.229474882.davem@davemloft.net>

David Miller a écrit :

> Eric, please make allocation failure cause the vlan_setup()
> to fail and propagate -EFAULT back to the caller.

Sure ! (You meant -ENOMEM probably...)

While testing my new patch in OOM situations, I found following bug
in vlan code, that gave me crashes or infinite loops.


[PATCH] vlan: Fix register_vlan_dev() error path

In case register_netdevice() returns an error, and a new vlan_group was
allocated and inserted in vlan_group_hash[] we call vlan_group_free() without
deleting group from hash table. Future lookups can give infinite loops or crashes.

We must delete the vlan_group using RCU safe procedure.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 net/8021q/vlan.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index 8836575..a29c5ab 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -281,8 +281,11 @@ out_uninit_applicant:
 	if (ngrp)
 		vlan_gvrp_uninit_applicant(real_dev);
 out_free_group:
-	if (ngrp)
-		vlan_group_free(ngrp);
+	if (ngrp) {
+		hlist_del_rcu(&ngrp->hlist);
+		/* Free the group, after all cpu's are done. */
+		call_rcu(&ngrp->rcu, vlan_rcu_free);
+	}
 	return err;
 }
 

^ permalink raw reply related

* Re: Shared i2c adapter locking
From: David Miller @ 2009-11-17 14:13 UTC (permalink / raw)
  To: bhutchings
  Cc: khali, sfr, netdev, linux-next, linux-kernel, mika.kuoppala,
	linux-i2c
In-Reply-To: <1258464774.2732.6.camel@achroite.uk.solarflarecom.com>

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Tue, 17 Nov 2009 13:32:54 +0000

> You will need to merge Linus's tree into net-next-2.6 and resolve the
> conflict by applying Jean's changes to drivers/net/sfc/falcon_boards.c.

Ok, I'll sort this out after I next push to Linus, thanks!

^ permalink raw reply

* Re: [RFC] mac80211: disallow bridging managed/adhoc interfaces
From: Johannes Berg @ 2009-11-17 14:07 UTC (permalink / raw)
  To: Michael Buesch; +Cc: netdev, linux-wireless, Stephen Hemminger, Felix Fietkau
In-Reply-To: <200911171504.54170.mb-fseUSCV1ubazQB+pC5nmwQ@public.gmane.org>

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

On Tue, 2009-11-17 at 15:04 +0100, Michael Buesch wrote:
> On Tuesday 17 November 2009 14:46:25 Johannes Berg wrote:
> > --- wireless-testing.orig/net/bridge/br_if.c	2009-11-17 14:19:17.000000000 +0100
> > +++ wireless-testing/net/bridge/br_if.c	2009-11-17 14:20:03.000000000 +0100
> > @@ -390,6 +390,10 @@ int br_add_if(struct net_bridge *br, str
> >  	if (dev->br_port != NULL)
> >  		return -EBUSY;
> >  
> > +	/* No bridging devices that dislike that (e.g. wireless) */
> > +	if (dev->priv_flags & IFF_DONT_BRIDGE)
> > +		return -EINVAL;
> 
> -EOPNOTSUPP?
> That would probably produce a better error message in userspace.

Good idea, will wait a bit for other comments and fix this when I send
as [PATCH].

johannes

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

^ permalink raw reply

* Re: [RFC] mac80211: disallow bridging managed/adhoc interfaces
From: John W. Linville @ 2009-11-17 14:06 UTC (permalink / raw)
  To: Michael Buesch
  Cc: Johannes Berg, netdev, linux-wireless, Stephen Hemminger,
	Felix Fietkau
In-Reply-To: <200911171504.54170.mb-fseUSCV1ubazQB+pC5nmwQ@public.gmane.org>

On Tue, Nov 17, 2009 at 03:04:52PM +0100, Michael Buesch wrote:
> On Tuesday 17 November 2009 14:46:25 Johannes Berg wrote:
> > --- wireless-testing.orig/net/bridge/br_if.c	2009-11-17 14:19:17.000000000 +0100
> > +++ wireless-testing/net/bridge/br_if.c	2009-11-17 14:20:03.000000000 +0100
> > @@ -390,6 +390,10 @@ int br_add_if(struct net_bridge *br, str
> >  	if (dev->br_port != NULL)
> >  		return -EBUSY;
> >  
> > +	/* No bridging devices that dislike that (e.g. wireless) */
> > +	if (dev->priv_flags & IFF_DONT_BRIDGE)
> > +		return -EINVAL;
> 
> -EOPNOTSUPP?
> That would probably produce a better error message in userspace.

Yes, good point.

-- 
John W. Linville		Someday the world will need a hero, and you
linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org			might be all we have.  Be ready.
--
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: [RFC] mac80211: disallow bridging managed/adhoc interfaces
From: Johannes Berg @ 2009-11-17 14:04 UTC (permalink / raw)
  To: John W. Linville; +Cc: netdev, linux-wireless, Stephen Hemminger, Felix Fietkau
In-Reply-To: <20091117135817.GC27743@tuxdriver.com>

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

On Tue, 2009-11-17 at 08:58 -0500, John W. Linville wrote:

> > To avoid such mistakes in the future, disallow adding a
> > wireless interface to a bridge.
> > 
> > Felix has recently added a four-address mode to the AP
> > and client side that can be used (after negotiating that
> > it is possible, which must happen out-of-band by setting
> > up both sides) for bridging, so allow that case.
> > 
> > Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
> 
> ACK -- we get these complaints fairly often...
> 
> I don't think I've sent the 4addr stuff to Dave yet.  Should I just
> take this through my tree?

After ack from Stephen maybe?

johannes

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

^ permalink raw reply

* Re: [RFC] mac80211: disallow bridging managed/adhoc interfaces
From: Michael Buesch @ 2009-11-17 14:04 UTC (permalink / raw)
  To: Johannes Berg; +Cc: netdev, linux-wireless, Stephen Hemminger, Felix Fietkau
In-Reply-To: <1258465585.3682.7.camel@johannes.local>

On Tuesday 17 November 2009 14:46:25 Johannes Berg wrote:
> --- wireless-testing.orig/net/bridge/br_if.c	2009-11-17 14:19:17.000000000 +0100
> +++ wireless-testing/net/bridge/br_if.c	2009-11-17 14:20:03.000000000 +0100
> @@ -390,6 +390,10 @@ int br_add_if(struct net_bridge *br, str
>  	if (dev->br_port != NULL)
>  		return -EBUSY;
>  
> +	/* No bridging devices that dislike that (e.g. wireless) */
> +	if (dev->priv_flags & IFF_DONT_BRIDGE)
> +		return -EINVAL;

-EOPNOTSUPP?
That would probably produce a better error message in userspace.

-- 
Greetings, Michael.

^ permalink raw reply

* Re: [RFC] mac80211: disallow bridging managed/adhoc interfaces
From: John W. Linville @ 2009-11-17 13:58 UTC (permalink / raw)
  To: Johannes Berg; +Cc: netdev, linux-wireless, Stephen Hemminger, Felix Fietkau
In-Reply-To: <1258465585.3682.7.camel-YfaajirXv2244ywRPIzf9A@public.gmane.org>

On Tue, Nov 17, 2009 at 02:46:25PM +0100, Johannes Berg wrote:
> A number of people have tried to add a wireless interface
> (in managed mode) to a bridge and then complained that it
> doesn't work. It cannot work, however, because in 802.11
> networks all packets need to be acknowledged and as such
> need to be sent to the right address. Promiscuous doesn't
> help here. The wireless address format used for these
> links has only space for three addresses, the
>  * transmitter, which must be equal to the sender (origin)
>  * receiver (on the wireless medium), which is the AP in
>    the case of managed mode
>  * the recipient (destination), which is on the APs local
>    network segment
> 
> In an IBSS, it is similar, but the receiver and recipient
> must match and the third address is used as the BSSID.
> 
> To avoid such mistakes in the future, disallow adding a
> wireless interface to a bridge.
> 
> Felix has recently added a four-address mode to the AP
> and client side that can be used (after negotiating that
> it is possible, which must happen out-of-band by setting
> up both sides) for bridging, so allow that case.
> 
> Signed-off-by: Johannes Berg <johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>

ACK -- we get these complaints fairly often...

I don't think I've sent the 4addr stuff to Dave yet.  Should I just
take this through my tree?

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org			might be all we have.  Be ready.
--
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] net/s390 drivers: add missing 'const' attribute
From: Heiko Carstens @ 2009-11-17 13:48 UTC (permalink / raw)
  To: David Miller; +Cc: Ursula Braun, netdev

Subject: [PATCH] net/s390 drivers: add missing 'const' attribute

From: Heiko Carstens <heiko.carstens@de.ibm.com>

Add missing 'const' attribute to avoid the following compile warnings:

drivers/s390/net/ctcm_main.c: In function 'ctcm_init':
drivers/s390/net/ctcm_main.c:1864: warning: assignment from incompatible pointer type
drivers/s390/net/lcs.c: In function 'lcs_init_module':
drivers/s390/net/lcs.c:2468: warning: assignment from incompatible pointer type
drivers/s390/net/claw.c: In function 'claw_init':
drivers/s390/net/claw.c:3408: warning: assignment from incompatible pointer type

Cc: Ursula Braun <braunu@de.ibm.com>
Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---

Patch applies on top of linux-next of 17th November.

 drivers/s390/net/claw.c      |    2 +-
 drivers/s390/net/ctcm_main.c |    2 +-
 drivers/s390/net/lcs.c       |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

Index: linux-next/drivers/s390/net/ctcm_main.c
===================================================================
--- linux-next.orig/drivers/s390/net/ctcm_main.c
+++ linux-next/drivers/s390/net/ctcm_main.c
@@ -1806,7 +1806,7 @@ static struct attribute_group ctcm_group
 	.attrs = ctcm_group_attrs,
 };
 
-static struct attribute_group *ctcm_group_attr_groups[] = {
+static const struct attribute_group *ctcm_group_attr_groups[] = {
 	&ctcm_group_attr_group,
 	NULL,
 };
Index: linux-next/drivers/s390/net/lcs.c
===================================================================
--- linux-next.orig/drivers/s390/net/lcs.c
+++ linux-next/drivers/s390/net/lcs.c
@@ -2440,7 +2440,7 @@ static struct attribute_group lcs_group_
 	.attrs = lcs_group_attrs,
 };
 
-static struct attribute_group *lcs_group_attr_groups[] = {
+static const struct attribute_group *lcs_group_attr_groups[] = {
 	&lcs_group_attr_group,
 	NULL,
 };
Index: linux-next/drivers/s390/net/claw.c
===================================================================
--- linux-next.orig/drivers/s390/net/claw.c
+++ linux-next/drivers/s390/net/claw.c
@@ -310,7 +310,7 @@ static struct attribute_group claw_group
 	.attrs = claw_group_attrs,
 };
 
-static struct attribute_group *claw_group_attr_groups[] = {
+static const struct attribute_group *claw_group_attr_groups[] = {
 	&claw_group_attr_group,
 	NULL,
 };

^ permalink raw reply

* [RFC] mac80211: disallow bridging managed/adhoc interfaces
From: Johannes Berg @ 2009-11-17 13:46 UTC (permalink / raw)
  To: netdev; +Cc: linux-wireless, Stephen Hemminger, Felix Fietkau

A number of people have tried to add a wireless interface
(in managed mode) to a bridge and then complained that it
doesn't work. It cannot work, however, because in 802.11
networks all packets need to be acknowledged and as such
need to be sent to the right address. Promiscuous doesn't
help here. The wireless address format used for these
links has only space for three addresses, the
 * transmitter, which must be equal to the sender (origin)
 * receiver (on the wireless medium), which is the AP in
   the case of managed mode
 * the recipient (destination), which is on the APs local
   network segment

In an IBSS, it is similar, but the receiver and recipient
must match and the third address is used as the BSSID.

To avoid such mistakes in the future, disallow adding a
wireless interface to a bridge.

Felix has recently added a four-address mode to the AP
and client side that can be used (after negotiating that
it is possible, which must happen out-of-band by setting
up both sides) for bridging, so allow that case.

Signed-off-by: Johannes Berg <johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>
---
 include/linux/if.h   |    1 +
 net/bridge/br_if.c   |    4 ++++
 net/mac80211/cfg.c   |    9 ++++++++-
 net/mac80211/iface.c |   17 +++++++++++++++--
 4 files changed, 28 insertions(+), 3 deletions(-)

--- wireless-testing.orig/include/linux/if.h	2009-11-17 14:18:36.000000000 +0100
+++ wireless-testing/include/linux/if.h	2009-11-17 14:19:04.000000000 +0100
@@ -70,6 +70,7 @@
 #define IFF_XMIT_DST_RELEASE 0x400	/* dev_hard_start_xmit() is allowed to
 					 * release skb->dst
 					 */
+#define IFF_DONT_BRIDGE 0x800		/* disallow bridging this ether dev */
 
 #define IF_GET_IFACE	0x0001		/* for querying only */
 #define IF_GET_PROTO	0x0002
--- wireless-testing.orig/net/bridge/br_if.c	2009-11-17 14:19:17.000000000 +0100
+++ wireless-testing/net/bridge/br_if.c	2009-11-17 14:20:03.000000000 +0100
@@ -390,6 +390,10 @@ int br_add_if(struct net_bridge *br, str
 	if (dev->br_port != NULL)
 		return -EBUSY;
 
+	/* No bridging devices that dislike that (e.g. wireless) */
+	if (dev->priv_flags & IFF_DONT_BRIDGE)
+		return -EINVAL;
+
 	p = new_nbp(br, dev);
 	if (IS_ERR(p))
 		return PTR_ERR(p);
--- wireless-testing.orig/net/mac80211/cfg.c	2009-11-17 14:21:24.000000000 +0100
+++ wireless-testing/net/mac80211/cfg.c	2009-11-17 14:37:13.000000000 +0100
@@ -106,8 +106,15 @@ static int ieee80211_change_iface(struct
 					    params->mesh_id_len,
 					    params->mesh_id);
 
-	if (params->use_4addr >= 0)
+	if (params->use_4addr >= 0) {
 		sdata->use_4addr = !!params->use_4addr;
+		sdata->dev->priv_flags &= ~IFF_DONT_BRIDGE;
+
+		if ((sdata->vif.type == NL80211_IFTYPE_STATION ||
+		     sdata->vif.type == NL80211_IFTYPE_ADHOC) &&
+		    !sdata->use_4addr)
+			sdata->dev->priv_flags |= IFF_DONT_BRIDGE;
+	}
 
 	if (sdata->vif.type != NL80211_IFTYPE_MONITOR || !flags)
 		return 0;
--- wireless-testing.orig/net/mac80211/iface.c	2009-11-17 14:20:19.000000000 +0100
+++ wireless-testing/net/mac80211/iface.c	2009-11-17 14:33:25.000000000 +0100
@@ -769,6 +769,11 @@ int ieee80211_if_change_type(struct ieee
 			sdata->local->hw.conf.channel->band);
 	sdata->drop_unencrypted = 0;
 	sdata->use_4addr = 0;
+	if (sdata->vif.type == NL80211_IFTYPE_STATION ||
+	    sdata->vif.type == NL80211_IFTYPE_ADHOC)
+		sdata->dev->priv_flags |= IFF_DONT_BRIDGE;
+	else
+		sdata->dev->priv_flags &= ~IFF_DONT_BRIDGE;
 
 	return 0;
 }
@@ -843,8 +848,16 @@ int ieee80211_if_add(struct ieee80211_lo
 					    params->mesh_id_len,
 					    params->mesh_id);
 
-	if (params && params->use_4addr >= 0)
-		sdata->use_4addr = !!params->use_4addr;
+	if (sdata->vif.type == NL80211_IFTYPE_STATION ||
+	    sdata->vif.type == NL80211_IFTYPE_ADHOC)
+		sdata->dev->priv_flags |= IFF_DONT_BRIDGE;
+	else
+		sdata->dev->priv_flags &= ~IFF_DONT_BRIDGE;
+
+	if (params && params->use_4addr > 0) {
+		sdata->use_4addr = true;
+		sdata->dev->priv_flags &= ~IFF_DONT_BRIDGE;
+	}
 
 	mutex_lock(&local->iflist_mtx);
 	list_add_tail_rcu(&sdata->list, &local->interfaces);


--
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: Shared i2c adapter locking
From: Ben Hutchings @ 2009-11-17 13:32 UTC (permalink / raw)
  To: David Miller
  Cc: khali, sfr, netdev, linux-next, linux-kernel, mika.kuoppala,
	linux-i2c
In-Reply-To: <20091117.035147.80164577.davem@davemloft.net>

On Tue, 2009-11-17 at 03:51 -0800, David Miller wrote:
> From: Jean Delvare <khali@linux-fr.org>
> Date: Tue, 17 Nov 2009 10:35:54 +0100
> 
> > If you did and something is still not clear, please let me know. My
> > understanding is that the action token is in David's hands now.
> 
> So what exactly do I need to do?
> 
> All I see that needs to happen is to pull from Linus's tree into
> net-2.6
> 
> But frankly I don't see how that helps resolve anything in linux-next
> since Stephen starts with Linus's tree then pulls in everyone's work.
> 
> Or is that pull necessary so that some other patch can be applied
> to net-2.6 or similar?

There is now a conflict between this in Linus's tree:

commit afa08974fe80c198b8650f73ed8ab59135ca10d0
Author: Jean Delvare <khali@linux-fr.org>
Date:   Sat Nov 7 13:10:46 2009 +0100

    i2c: Add an interface to lock/unlock an I2C bus segment

and this in net-next-2.6:

commit c9597d4f89565b6562bd3026adbe6eac6c317f47
Author: Ben Hutchings <bhutchings@solarflare.com>
Date:   Fri Oct 23 08:29:33 2009 +0000

    sfc: Merge sfe4001.c into falcon_boards.c

You will need to merge Linus's tree into net-next-2.6 and resolve the
conflict by applying Jean's changes to drivers/net/sfc/falcon_boards.c.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* Re: gro: Fix illegal merging of trailer trash
From: David Miller @ 2009-11-17 13:18 UTC (permalink / raw)
  To: herbert; +Cc: netdev, mwagner
In-Reply-To: <20091117124414.GA14951@gondor.apana.org.au>

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Tue, 17 Nov 2009 20:44:14 +0800

> gro: Fix illegal merging of trailer trash
> 
> When we've merged skb's with page frags, and subsequently receive
> a trailer skb (< MSS) that is not completely non-linear (this can
> occur on Intel NICs if the packet size falls below the threshold),
> GRO ends up producing an illegal GSO skb with a frag_list.
> 
> This is harmless unless the skb is then forwarded through an
> interface that requires software GSO, whereupon the GSO code
> will BUG.
> 
> This patch detects this case in GRO and avoids merging the
> trailer skb.
> 
> Reported-by: Mark Wagner <mwagner@redhat.com>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Applied, and queued up for -stable, thanks.

^ permalink raw reply

* Re: updates to the mscan-driver in net-next
From: Wolfram Sang @ 2009-11-17 13:07 UTC (permalink / raw)
  To: netdev; +Cc: socketcan-core, linuxppc-dev, David Miller
In-Reply-To: <1258412274-14686-1-git-send-email-w.sang@pengutronix.de>

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

> I will also update my branch on pengutronix.de tomorrow,

Done.

===

The following changes since commit a0a9020c1725cd5c9a13a7aab65831f3c85ea9ca:
  Wolfram Sang (1):
        net/can/mscan: final checkpatch cleanups

are available in the git repository at:

  git://git.pengutronix.de/git/wsa/linux-2.6.git net-next-52xx-can

Wolfram Sang (11):
      net/can/mscan: move defines into .h file
      net/can/mscan: trivial fixes
      net/can/mscan: drop support for CAN_MODE_{SLEEP|STOP}
      net/can/mscan: use {clr|set}bits8 macros
      net/can/mscan: fix function annotations
      net/can/mscan: drop assignment in while-construct
      net/can/mpc52xx_can: refactor clock-get routine
      net/can/mpc52xx_can: improve properties and their description
      net/can/mscan: replace hardcoded values with defines
      net/can/mscan: add error path to mscan_open()
      net/can/mscan: improve build

 Documentation/powerpc/dts-bindings/fsl/mpc5200.txt |    9 +-
 drivers/net/can/Kconfig                            |   19 +---
 drivers/net/can/mscan/Kconfig                      |   23 ++++
 drivers/net/can/mscan/Makefile                     |    4 +-
 .../net/can/mscan/{mpc52xx_can.c => mpc5xxx_can.c} |   64 ++++-------
 drivers/net/can/mscan/mscan.c                      |  121 +++++++------------
 drivers/net/can/mscan/mscan.h                      |   36 ++++++-
 7 files changed, 133 insertions(+), 143 deletions(-)
 create mode 100644 drivers/net/can/mscan/Kconfig
 rename drivers/net/can/mscan/{mpc52xx_can.c => mpc5xxx_can.c} (90%)

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

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

^ permalink raw reply

* [RFC] [PATCH net-next] Phonet: convert devices list to RCU
From: Rémi Denis-Courmont @ 2009-11-17 13:02 UTC (permalink / raw)
  To: netdev; +Cc: Rémi Denis-Courmont

From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>

Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
---
 include/net/phonet/pn_dev.h |    2 +-
 net/phonet/pn_dev.c         |   63 ++++++++++++++++++++++++++++---------------
 net/phonet/pn_netlink.c     |    6 ++--
 3 files changed, 45 insertions(+), 26 deletions(-)

diff --git a/include/net/phonet/pn_dev.h b/include/net/phonet/pn_dev.h
index afa7def..d7b989c 100644
--- a/include/net/phonet/pn_dev.h
+++ b/include/net/phonet/pn_dev.h
@@ -25,7 +25,7 @@
 
 struct phonet_device_list {
 	struct list_head list;
-	spinlock_t lock;
+	struct mutex lock;
 };
 
 struct phonet_device_list *phonet_device_list(struct net *net);
diff --git a/net/phonet/pn_dev.c b/net/phonet/pn_dev.c
index 3287f8f..3d9608f 100644
--- a/net/phonet/pn_dev.c
+++ b/net/phonet/pn_dev.c
@@ -61,7 +61,8 @@ static struct phonet_device *__phonet_device_alloc(struct net_device *dev)
 	pnd->netdev = dev;
 	bitmap_zero(pnd->addrs, 64);
 
-	list_add(&pnd->list, &pndevs->list);
+	BUG_ON(!mutex_is_locked(&pndevs->lock));
+	list_add_rcu(&pnd->list, &pndevs->list);
 	return pnd;
 }
 
@@ -70,6 +71,7 @@ static struct phonet_device *__phonet_get(struct net_device *dev)
 	struct phonet_device_list *pndevs = phonet_device_list(dev_net(dev));
 	struct phonet_device *pnd;
 
+	BUG_ON(!mutex_is_locked(&pndevs->lock));
 	list_for_each_entry(pnd, &pndevs->list, list) {
 		if (pnd->netdev == dev)
 			return pnd;
@@ -77,6 +79,18 @@ static struct phonet_device *__phonet_get(struct net_device *dev)
 	return NULL;
 }
 
+static struct phonet_device *__phonet_get_rcu(struct net_device *dev)
+{
+	struct phonet_device_list *pndevs = phonet_device_list(dev_net(dev));
+	struct phonet_device *pnd;
+
+	list_for_each_entry_rcu(pnd, &pndevs->list, list) {
+		if (pnd->netdev == dev)
+			return pnd;
+	}
+	return NULL;
+}
+
 static void phonet_device_destroy(struct net_device *dev)
 {
 	struct phonet_device_list *pndevs = phonet_device_list(dev_net(dev));
@@ -84,11 +98,11 @@ static void phonet_device_destroy(struct net_device *dev)
 
 	ASSERT_RTNL();
 
-	spin_lock_bh(&pndevs->lock);
+	mutex_lock(&pndevs->lock);
 	pnd = __phonet_get(dev);
 	if (pnd)
-		list_del(&pnd->list);
-	spin_unlock_bh(&pndevs->lock);
+		list_del_rcu(&pnd->list);
+	mutex_unlock(&pndevs->lock);
 
 	if (pnd) {
 		u8 addr;
@@ -106,8 +120,8 @@ struct net_device *phonet_device_get(struct net *net)
 	struct phonet_device *pnd;
 	struct net_device *dev = NULL;
 
-	spin_lock_bh(&pndevs->lock);
-	list_for_each_entry(pnd, &pndevs->list, list) {
+	rcu_read_lock();
+	list_for_each_entry_rcu(pnd, &pndevs->list, list) {
 		dev = pnd->netdev;
 		BUG_ON(!dev);
 
@@ -118,7 +132,7 @@ struct net_device *phonet_device_get(struct net *net)
 	}
 	if (dev)
 		dev_hold(dev);
-	spin_unlock_bh(&pndevs->lock);
+	rcu_read_unlock();
 	return dev;
 }
 
@@ -128,7 +142,7 @@ int phonet_address_add(struct net_device *dev, u8 addr)
 	struct phonet_device *pnd;
 	int err = 0;
 
-	spin_lock_bh(&pndevs->lock);
+	mutex_lock(&pndevs->lock);
 	/* Find or create Phonet-specific device data */
 	pnd = __phonet_get(dev);
 	if (pnd == NULL)
@@ -137,7 +151,7 @@ int phonet_address_add(struct net_device *dev, u8 addr)
 		err = -ENOMEM;
 	else if (test_and_set_bit(addr >> 2, pnd->addrs))
 		err = -EEXIST;
-	spin_unlock_bh(&pndevs->lock);
+	mutex_unlock(&pndevs->lock);
 	return err;
 }
 
@@ -147,27 +161,32 @@ int phonet_address_del(struct net_device *dev, u8 addr)
 	struct phonet_device *pnd;
 	int err = 0;
 
-	spin_lock_bh(&pndevs->lock);
+	mutex_lock(&pndevs->lock);
 	pnd = __phonet_get(dev);
-	if (!pnd || !test_and_clear_bit(addr >> 2, pnd->addrs))
+	if (!pnd || !test_and_clear_bit(addr >> 2, pnd->addrs)) {
 		err = -EADDRNOTAVAIL;
-	else if (bitmap_empty(pnd->addrs, 64)) {
-		list_del(&pnd->list);
+		pnd = NULL;
+	} else if (bitmap_empty(pnd->addrs, 64))
+		list_del_rcu(&pnd->list);
+	else
+		pnd = NULL;
+	mutex_unlock(&pndevs->lock);
+
+	if (pnd) {
+		synchronize_rcu();
 		kfree(pnd);
 	}
-	spin_unlock_bh(&pndevs->lock);
 	return err;
 }
 
 /* Gets a source address toward a destination, through a interface. */
 u8 phonet_address_get(struct net_device *dev, u8 daddr)
 {
-	struct phonet_device_list *pndevs = phonet_device_list(dev_net(dev));
 	struct phonet_device *pnd;
 	u8 saddr;
 
-	spin_lock_bh(&pndevs->lock);
-	pnd = __phonet_get(dev);
+	rcu_read_lock();
+	pnd = __phonet_get_rcu(dev);
 	if (pnd) {
 		BUG_ON(bitmap_empty(pnd->addrs, 64));
 
@@ -178,7 +197,7 @@ u8 phonet_address_get(struct net_device *dev, u8 daddr)
 			saddr = find_first_bit(pnd->addrs, 64) << 2;
 	} else
 		saddr = PN_NO_ADDR;
-	spin_unlock_bh(&pndevs->lock);
+	rcu_read_unlock();
 
 	if (saddr == PN_NO_ADDR) {
 		/* Fallback to another device */
@@ -200,8 +219,8 @@ int phonet_address_lookup(struct net *net, u8 addr)
 	struct phonet_device *pnd;
 	int err = -EADDRNOTAVAIL;
 
-	spin_lock_bh(&pndevs->lock);
-	list_for_each_entry(pnd, &pndevs->list, list) {
+	rcu_read_lock();
+	list_for_each_entry_rcu(pnd, &pndevs->list, list) {
 		/* Don't allow unregistering devices! */
 		if ((pnd->netdev->reg_state != NETREG_REGISTERED) ||
 				((pnd->netdev->flags & IFF_UP)) != IFF_UP)
@@ -213,7 +232,7 @@ int phonet_address_lookup(struct net *net, u8 addr)
 		}
 	}
 found:
-	spin_unlock_bh(&pndevs->lock);
+	rcu_read_unlock();
 	return err;
 }
 
@@ -304,7 +323,7 @@ static int phonet_init_net(struct net *net)
 	}
 
 	INIT_LIST_HEAD(&pnn->pndevs.list);
-	spin_lock_init(&pnn->pndevs.lock);
+	mutex_init(&pnn->pndevs.lock);
 	mutex_init(&pnn->routes.lock);
 	net_assign_generic(net, phonet_net_id, pnn);
 	return 0;
diff --git a/net/phonet/pn_netlink.c b/net/phonet/pn_netlink.c
index 609e509..2e6c7eb 100644
--- a/net/phonet/pn_netlink.c
+++ b/net/phonet/pn_netlink.c
@@ -131,8 +131,8 @@ static int getaddr_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
 	int addr_idx = 0, addr_start_idx = cb->args[1];
 
 	pndevs = phonet_device_list(sock_net(skb->sk));
-	spin_lock_bh(&pndevs->lock);
-	list_for_each_entry(pnd, &pndevs->list, list) {
+	rcu_read_lock();
+	list_for_each_entry_rcu(pnd, &pndevs->list, list) {
 		u8 addr;
 
 		if (dev_idx > dev_start_idx)
@@ -154,7 +154,7 @@ static int getaddr_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
 	}
 
 out:
-	spin_unlock_bh(&pndevs->lock);
+	rcu_read_unlock();
 	cb->args[0] = dev_idx;
 	cb->args[1] = addr_idx;
 
-- 
1.6.3.3


^ permalink raw reply related

* Re: [net-next-2.6 PATCH v6 4/7 RFC] TCPCT part 1d: define TCP cookie option, extend existing struct's
From: David Miller @ 2009-11-17 12:48 UTC (permalink / raw)
  To: ilpo.jarvinen; +Cc: eric.dumazet, william.allen.simpson, netdev, joe
In-Reply-To: <alpine.DEB.2.00.0911171432420.7024@wel-95.cs.helsinki.fi>

From: "Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi>
Date: Tue, 17 Nov 2009 14:38:42 +0200 (EET)

> Right, but we could be just as crazy and just drop such things and
> keep resending SYN+DATA without any harm?

Two crazies == sane? :-)

Sure, we could do that.

^ permalink raw reply

* gro: Fix illegal merging of trailer trash
From: Herbert Xu @ 2009-11-17 12:44 UTC (permalink / raw)
  To: David S. Miller, netdev; +Cc: Mark Wagner

Hi Dave:

Just found this thanks to Mark Wagner testing GRO with Xen.
We'll need this for stable too.

gro: Fix illegal merging of trailer trash

When we've merged skb's with page frags, and subsequently receive
a trailer skb (< MSS) that is not completely non-linear (this can
occur on Intel NICs if the packet size falls below the threshold),
GRO ends up producing an illegal GSO skb with a frag_list.

This is harmless unless the skb is then forwarded through an
interface that requires software GSO, whereupon the GSO code
will BUG.

This patch detects this case in GRO and avoids merging the
trailer skb.

Reported-by: Mark Wagner <mwagner@redhat.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 80a9616..ec85681 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2701,7 +2701,8 @@ int skb_gro_receive(struct sk_buff **head, struct sk_buff *skb)
 
 		NAPI_GRO_CB(skb)->free = 1;
 		goto done;
-	}
+	} else if (skb_gro_len(p) != pinfo->gso_size)
+		return -E2BIG;
 
 	headroom = skb_headroom(p);
 	nskb = netdev_alloc_skb(p->dev, headroom + skb_gro_offset(p));

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply related

* Re: [net-next-2.6 PATCH v6 4/7 RFC] TCPCT part 1d: define TCP cookie option, extend existing struct's
From: Ilpo Järvinen @ 2009-11-17 12:38 UTC (permalink / raw)
  To: David Miller; +Cc: eric.dumazet, william.allen.simpson, Netdev, joe
In-Reply-To: <20091117.042250.28821014.davem@davemloft.net>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1402 bytes --]

On Tue, 17 Nov 2009, David Miller wrote:

> From: "Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi>
> Date: Tue, 17 Nov 2009 14:18:57 +0200 (EET)
> 
> > On Mon, 16 Nov 2009, David Miller wrote:
> > 
> >> From: Eric Dumazet <eric.dumazet@gmail.com>
> >> Date: Mon, 16 Nov 2009 23:26:04 +0100
> >> 
> >> > So adding DATA to SYN packets might be problematic for part of our tcp 
> >> > stack. 
> >> 
> >> I can almost guarentee it won't work.  For one thing getting a SACK
> >> response to a SYN+DATA packet will explode quite nicely for one thing.
> > 
> > Now I'm really lost??? How can you get SACKs for that in the first 
> > place since they are either lost or delivered in unison???
> 
> Ideally, you're probably right.
> 
> However, it seems to me that the receiver can do whatever it likes
> with it's receive queue when it's under memory pressure.
> 
> It can chop packets up, partially free bits, and then send a SACK
> block back to you for the parts it tried to free.
> 
> If you'll recall, I wanted to put some tough restrictions into what is
> allowed with SACK so that we could optimize things on the sender side.
> But there was resistence and therefore we have to keep allowing all
> kinds of silly situations, the one we're talking about here merely
> being one of them :-)

Right, but we could be just as crazy and just drop such things and keep 
resending SYN+DATA without any harm?

-- 
 i.

^ permalink raw reply

* Re: [RFC PATCH] net: add dataref destructor to sk_buff
From: Gregory Haskins @ 2009-11-17 12:33 UTC (permalink / raw)
  To: Herbert Xu
  Cc: David Miller, ghaskins, mst, alacrityvm-devel, linux-kernel,
	netdev
In-Reply-To: <20091117010238.GA10029@gondor.apana.org.au>

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

Herbert Xu wrote:
> On Mon, Nov 16, 2009 at 09:22:57AM -0500, Gregory Haskins wrote:
>> But really, this is somewhat orthogonal to the original problem, so let
>> me see if we can bring it back on topic.  Michael stated that this patch
>> in question may be problematic because there are places in the stack
>> that can get_page() without also maintaining a reference to the shinfo
>> object.  Evgeniy seems to say the opposite.  I am not sure who is right,
>> or if I misunderstood one or both of them.  Any thoughts?
> 
> There are loads of places where this can happen.  Start with
> pskb_expand_head.

Indeed, I see your point.  Looks like we can potentially solve that with
an extra level of indirection.  E.g. skb->shinfo->owner, with a
ref-count+callback.  Thoughts?

Kind Regards,
-Greg



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 267 bytes --]

^ permalink raw reply

* Re: PATCH net-next-2.6] linkwatch: linkwatch_forget_dev() to speedup device dismantle
From: Herbert Xu @ 2009-11-17 12:31 UTC (permalink / raw)
  To: David Miller; +Cc: shemminger, eric.dumazet, netdev
In-Reply-To: <20091117.042604.99618154.davem@davemloft.net>

On Tue, Nov 17, 2009 at 04:26:04AM -0800, David Miller wrote:
>
> Really, the link watch stuff is just due for a redesign.  I don't
> think a simple hack is going to cut it this time, sorry Eric :-)

I have no objections against any redesigns, but since the only
caller of linkwatch_forget_dev runs in process context with the
RTNL, it could also legally emit those events.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: PATCH net-next-2.6] linkwatch: linkwatch_forget_dev() to speedup device dismantle
From: David Miller @ 2009-11-17 12:26 UTC (permalink / raw)
  To: shemminger; +Cc: eric.dumazet, herbert, netdev
In-Reply-To: <20091116143917.14e4fa4f@s6510>

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Mon, 16 Nov 2009 14:39:17 -0800

> On Mon, 16 Nov 2009 22:50:48 +0100
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
> 
>> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> 
> Acked-by: Stephen Hemminger <shemminger@vyatta.com>

Is this really valid?

The whole point in emitting the netif_carrier_off() is so
that applications see the event in userspace and therefore
can clean things up.

Sure, the kernel will no longer make the device visible, and therefore
the application can't operate on it any longer.  But the application
is deserved of receiving the event anyways so that it can clean up
internal state and datastructures.

It seem to me that in this ->stop() case we'll now elide the event
emission, and I don't see how that can be right.

Really, the link watch stuff is just due for a redesign.  I don't
think a simple hack is going to cut it this time, sorry Eric :-)

^ permalink raw reply

* Re: [net-next-2.6 PATCH v6 4/7 RFC] TCPCT part 1d: define TCP cookie option, extend existing struct's
From: David Miller @ 2009-11-17 12:22 UTC (permalink / raw)
  To: ilpo.jarvinen; +Cc: eric.dumazet, william.allen.simpson, netdev, joe
In-Reply-To: <alpine.DEB.2.00.0911171412380.7024@wel-95.cs.helsinki.fi>

From: "Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi>
Date: Tue, 17 Nov 2009 14:18:57 +0200 (EET)

> On Mon, 16 Nov 2009, David Miller wrote:
> 
>> From: Eric Dumazet <eric.dumazet@gmail.com>
>> Date: Mon, 16 Nov 2009 23:26:04 +0100
>> 
>> > So adding DATA to SYN packets might be problematic for part of our tcp 
>> > stack. 
>> 
>> I can almost guarentee it won't work.  For one thing getting a SACK
>> response to a SYN+DATA packet will explode quite nicely for one thing.
> 
> Now I'm really lost??? How can you get SACKs for that in the first 
> place since they are either lost or delivered in unison???

Ideally, you're probably right.

However, it seems to me that the receiver can do whatever it likes
with it's receive queue when it's under memory pressure.

It can chop packets up, partially free bits, and then send a SACK
block back to you for the parts it tried to free.

If you'll recall, I wanted to put some tough restrictions into what is
allowed with SACK so that we could optimize things on the sender side.
But there was resistence and therefore we have to keep allowing all
kinds of silly situations, the one we're talking about here merely
being one of them :-)

^ permalink raw reply

* Re: [net-next-2.6 PATCH v6 4/7 RFC] TCPCT part 1d: define TCP cookie option, extend existing struct's
From: Ilpo Järvinen @ 2009-11-17 12:18 UTC (permalink / raw)
  To: David Miller; +Cc: eric.dumazet, william.allen.simpson, Netdev, joe
In-Reply-To: <20091116.191522.178198674.davem@davemloft.net>

On Mon, 16 Nov 2009, David Miller wrote:

> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Mon, 16 Nov 2009 23:26:04 +0100
> 
> > So adding DATA to SYN packets might be problematic for part of our tcp 
> > stack. 
> 
> I can almost guarentee it won't work.  For one thing getting a SACK
> response to a SYN+DATA packet will explode quite nicely for one thing.

Now I'm really lost??? How can you get SACKs for that in the first 
place since they are either lost or delivered in unison???

> A lot of the other retransmit queue handling would need to be audited
> as well.  So much code assumes that if we see sent data in the
> retransmit queue, there won't be SYN or SYN+ACK things in there to
> contend with.

...Valid ACKs will either remove that SYN+DATA segment completely, other 
ACK could (or should) just be dropped. I kind of fail to see what is the 
problem here.

-- 
 i.

^ permalink raw reply

* Re: [PATCH 2/2] act_mirred: optimization
From: David Miller @ 2009-11-17 12:16 UTC (permalink / raw)
  To: xiaosuo; +Cc: hadi, shemminger, netdev
In-Reply-To: <412e6f7f0911162149q1eedf18vfbf980fc690629f5@mail.gmail.com>

From: Changli Gao <xiaosuo@gmail.com>
Date: Tue, 17 Nov 2009 13:49:00 +0800

> On Mon, Nov 16, 2009 at 9:33 PM, David Miller <davem@davemloft.net> wrote:
>> From: Changli Gao <xiaosuo@gmail.com>
>> Date: Mon, 16 Nov 2009 21:22:46 +0800
>>
>>> I am working against linux-next. I'll check if there is any
>>> difference. BTW: were these patches applied in order, 1 - 2?
>>
>> Yes, they were.
>>
> 
> Patch is resubmitted as attachment.

Also applied, thanks.

^ 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