Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] sctp: Make "Invalid Stream Identifier" ERROR follows SACK when bundling
From: David Miller @ 2012-07-22 19:45 UTC (permalink / raw)
  To: xufengzhang.main; +Cc: vyasevich, sri, linux-sctp, netdev, linux-kernel
In-Reply-To: <1342677450-21810-1-git-send-email-xufengzhang.main@gmail.com>

From: <xufengzhang.main@gmail.com>
Date: Thu, 19 Jul 2012 13:57:30 +0800

> When "Invalid Stream Identifier" ERROR happens after process the
> received DATA chunks, this ERROR chunk is enqueued into outqueue
> before SACK chunk, so when bundling ERROR chunk with SACK chunk,
> the ERROR chunk is always placed first in the packet because of
> the chunk's position in the outqueue.
> This violates sctp specification:
>     RFC 4960 6.5. Stream Identifier and Stream Sequence Number
>     ...The endpoint may bundle the ERROR chunk in the same
>     packet as the SACK as long as the ERROR follows the SACK.
> So we must place SACK first when bundling "Invalid Stream Identifier"
> ERROR and SACK in one packet.
> Although we can do that by enqueue SACK chunk into outqueue before
> ERROR chunk, it will violate the side-effect interpreter processing.
> It's easy to do this job when dequeue chunks from the outqueue,
> by this way, we introduce a flag 'has_isi_err' which indicate
> whether or not the "Invalid Stream Identifier" ERROR happens.
> 
> Signed-off-by: Xufeng Zhang <xufeng.zhang@windriver.com>

Can some SCTP experts please review this?

^ permalink raw reply

* Re: [PATCH v3 0/2] net: ethernet: davinci_emac: fixups + pm_runtime
From: David Miller @ 2012-07-22 19:46 UTC (permalink / raw)
  To: mgreer; +Cc: netdev, linux-omap, linux-arm-kernel
In-Reply-To: <1342833562-3435-1-git-send-email-mgreer@animalcreek.com>

From: "Mark A. Greer" <mgreer@animalcreek.com>
Date: Fri, 20 Jul 2012 18:19:20 -0700

> From: "Mark A. Greer" <mgreer@animalcreek.com>
> 
> This series fixes a compile error in, and adds pm_runtime support
> to, the davinci_emac driver.
> 
> To test on a davinci platform, you will need another patch just
> submitted to netdev:
> 
> 	http://marc.info/?l=linux-netdev&m=134282758408187&w=2
> 
> Mark A. Greer (2):
>   net: ethernet: davinci_emac: Remove unnecessary #include
>   net: ethernet: davinci_emac: add pm_runtime support

All applied to net-next, thanks.

^ permalink raw reply

* [PATCH] ppp: add 64 bit stats
From: Kevin Groeneveld @ 2012-07-22 20:19 UTC (permalink / raw)
  To: netdev; +Cc: Kevin Groeneveld

Add 64 bit stats to ppp driver.  The 64 bit stats include tx_bytes,
rx_bytes, tx_packets and rx_packets.  Other stats are still 32 bit.
The 64 bit stats can be retrieved via the ndo_get_stats operation.  The
SIOCGPPPSTATS ioctl is still 32 bit stats only.

Signed-off-by: Kevin Groeneveld <kgroeneveld@gmail.com>
---
 drivers/net/ppp/ppp_generic.c |  110 +++++++++++++++++++++++++++++++++++------
 1 file changed, 95 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
index 5c05572..210238c 100644
--- a/drivers/net/ppp/ppp_generic.c
+++ b/drivers/net/ppp/ppp_generic.c
@@ -94,6 +94,19 @@ struct ppp_file {
 #define PF_TO_CHANNEL(pf)	PF_TO_X(pf, struct channel)
 
 /*
+ * Data structure to hold primary network stats for which
+ * we want to use 64 bit storage.  Other network stats
+ * are stored in dev->stats of the ppp strucute.
+ */
+struct ppp_link_stats {
+	struct u64_stats_sync syncp;
+	u64 tx_bytes;
+	u64 tx_packets;
+	u64 rx_bytes;
+	u64 rx_packets;
+};
+
+/*
  * Data structure describing one ppp unit.
  * A ppp unit corresponds to a ppp network interface device
  * and represents a multilink bundle.
@@ -136,6 +149,7 @@ struct ppp {
 	unsigned pass_len, active_len;
 #endif /* CONFIG_PPP_FILTER */
 	struct net	*ppp_net;	/* the net we belong to */
+	struct ppp_link_stats __percpu *stats;	/* 64 bit network stats */
 };
 
 /*
@@ -1021,9 +1035,45 @@ ppp_net_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 	return err;
 }
 
+struct rtnl_link_stats64*
+ppp_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *tot)
+{
+	struct ppp *ppp = netdev_priv(dev);
+	int cpu;
+
+	for_each_possible_cpu(cpu) {
+		struct ppp_link_stats *stats = per_cpu_ptr(ppp->stats, cpu);
+		unsigned int start;
+		u64 rx_packets, rx_bytes, tx_packets, tx_bytes;
+
+		do {
+			start = u64_stats_fetch_begin_bh(&stats->syncp);
+			rx_packets = stats->rx_packets;
+			tx_packets = stats->tx_packets;
+			rx_bytes   = stats->rx_bytes;
+			tx_bytes   = stats->tx_bytes;
+
+		} while (u64_stats_fetch_retry_bh(&stats->syncp, start));
+
+		tot->rx_packets += rx_packets;
+		tot->tx_packets += tx_packets;
+		tot->rx_bytes   += rx_bytes;
+		tot->tx_bytes   += tx_bytes;
+	}
+
+	tot->rx_errors        = dev->stats.rx_errors;
+	tot->tx_errors        = dev->stats.tx_errors;
+	tot->rx_dropped       = dev->stats.rx_dropped;
+	tot->tx_dropped       = dev->stats.tx_dropped;
+	tot->rx_length_errors = dev->stats.rx_length_errors;
+
+	return tot;
+}
+
 static const struct net_device_ops ppp_netdev_ops = {
-	.ndo_start_xmit = ppp_start_xmit,
-	.ndo_do_ioctl   = ppp_net_ioctl,
+	.ndo_start_xmit  = ppp_start_xmit,
+	.ndo_do_ioctl    = ppp_net_ioctl,
+	.ndo_get_stats64 = ppp_get_stats64,
 };
 
 static void ppp_setup(struct net_device *dev)
@@ -1130,6 +1180,7 @@ ppp_send_frame(struct ppp *ppp, struct sk_buff *skb)
 	struct sk_buff *new_skb;
 	int len;
 	unsigned char *cp;
+	struct ppp_link_stats *stats = this_cpu_ptr(ppp->stats);
 
 	if (proto < 0x8000) {
 #ifdef CONFIG_PPP_FILTER
@@ -1157,8 +1208,10 @@ ppp_send_frame(struct ppp *ppp, struct sk_buff *skb)
 #endif /* CONFIG_PPP_FILTER */
 	}
 
-	++ppp->dev->stats.tx_packets;
-	ppp->dev->stats.tx_bytes += skb->len - 2;
+	u64_stats_update_begin(&stats->syncp);
+	++stats->tx_packets;
+	stats->tx_bytes += skb->len - 2;
+	u64_stats_update_end(&stats->syncp);
 
 	switch (proto) {
 	case PPP_IP:
@@ -1673,6 +1726,7 @@ ppp_receive_nonmp_frame(struct ppp *ppp, struct sk_buff *skb)
 {
 	struct sk_buff *ns;
 	int proto, len, npi;
+	struct ppp_link_stats *stats = this_cpu_ptr(ppp->stats);
 
 	/*
 	 * Decompress the frame, if compressed.
@@ -1745,8 +1799,10 @@ ppp_receive_nonmp_frame(struct ppp *ppp, struct sk_buff *skb)
 		break;
 	}
 
-	++ppp->dev->stats.rx_packets;
-	ppp->dev->stats.rx_bytes += skb->len - 2;
+	u64_stats_update_begin(&stats->syncp);
+	++stats->rx_packets;
+	stats->rx_bytes += skb->len - 2;
+	u64_stats_update_end(&stats->syncp);
 
 	npi = proto_to_npindex(proto);
 	if (npi < 0) {
@@ -2568,14 +2624,32 @@ static void
 ppp_get_stats(struct ppp *ppp, struct ppp_stats *st)
 {
 	struct slcompress *vj = ppp->vj;
+	int cpu;
 
 	memset(st, 0, sizeof(*st));
-	st->p.ppp_ipackets = ppp->dev->stats.rx_packets;
+
+	for_each_possible_cpu(cpu) {
+		struct ppp_link_stats *stats = per_cpu_ptr(ppp->stats, cpu);
+		unsigned int start;
+		u64 rx_packets, rx_bytes, tx_packets, tx_bytes;
+
+		do {
+			start = u64_stats_fetch_begin_bh(&stats->syncp);
+			rx_packets = stats->rx_packets;
+			tx_packets = stats->tx_packets;
+			rx_bytes   = stats->rx_bytes;
+			tx_bytes   = stats->tx_bytes;
+
+		} while (u64_stats_fetch_retry_bh(&stats->syncp, start));
+
+		st->p.ppp_ipackets += rx_packets;
+		st->p.ppp_opackets += tx_packets;
+		st->p.ppp_ibytes   += rx_bytes;
+		st->p.ppp_obytes   += tx_bytes;
+	}
+
 	st->p.ppp_ierrors = ppp->dev->stats.rx_errors;
-	st->p.ppp_ibytes = ppp->dev->stats.rx_bytes;
-	st->p.ppp_opackets = ppp->dev->stats.tx_packets;
 	st->p.ppp_oerrors = ppp->dev->stats.tx_errors;
-	st->p.ppp_obytes = ppp->dev->stats.tx_bytes;
 	if (!vj)
 		return;
 	st->vj.vjs_packets = vj->sls_o_compressed + vj->sls_o_uncompressed;
@@ -2627,6 +2701,9 @@ ppp_create_interface(struct net *net, int unit, int *retp)
 	ppp->minseq = -1;
 	skb_queue_head_init(&ppp->mrq);
 #endif /* CONFIG_PPP_MULTILINK */
+	ppp->stats = alloc_percpu(struct ppp_link_stats);
+	if (ppp->stats == NULL)
+		goto out2;
 
 	/*
 	 * drum roll: don't forget to set
@@ -2640,12 +2717,12 @@ ppp_create_interface(struct net *net, int unit, int *retp)
 		unit = unit_get(&pn->units_idr, ppp);
 		if (unit < 0) {
 			ret = unit;
-			goto out2;
+			goto out3;
 		}
 	} else {
 		ret = -EEXIST;
 		if (unit_find(&pn->units_idr, unit))
-			goto out2; /* unit already exists */
+			goto out3; /* unit already exists */
 		/*
 		 * if caller need a specified unit number
 		 * lets try to satisfy him, otherwise --
@@ -2657,7 +2734,7 @@ ppp_create_interface(struct net *net, int unit, int *retp)
 		 */
 		unit = unit_set(&pn->units_idr, ppp, unit);
 		if (unit < 0)
-			goto out2;
+			goto out3;
 	}
 
 	/* Initialize the new ppp unit */
@@ -2669,7 +2746,7 @@ ppp_create_interface(struct net *net, int unit, int *retp)
 		unit_put(&pn->units_idr, unit);
 		netdev_err(ppp->dev, "PPP: couldn't register device %s (%d)\n",
 			   dev->name, ret);
-		goto out2;
+		goto out3;
 	}
 
 	ppp->ppp_net = net;
@@ -2680,8 +2757,10 @@ ppp_create_interface(struct net *net, int unit, int *retp)
 	*retp = 0;
 	return ppp;
 
-out2:
+out3:
 	mutex_unlock(&pn->all_ppp_mutex);
+	free_percpu(ppp->stats);
+out2:
 	free_netdev(dev);
 out1:
 	*retp = ret;
@@ -2765,6 +2844,7 @@ static void ppp_destroy_interface(struct ppp *ppp)
 
 	kfree_skb(ppp->xmit_pending);
 
+	free_percpu(ppp->stats);
 	free_netdev(ppp->dev);
 }
 
-- 
1.7.9.5

^ permalink raw reply related

* Re: [PATCH RFC] tcp: use seqlock for all cached tcp_metrics
From: Julian Anastasov @ 2012-07-22 20:34 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20120722.123246.864281319907290494.davem@davemloft.net>


	Hello,

On Sun, 22 Jul 2012, David Miller wrote:

> From: Julian Anastasov <ja@ssi.bg>
> Date: Sun, 22 Jul 2012 12:44:28 +0300
> 
> > 	The ability to reclaim existing cache entries
> > requires metrics to be accessed with additional seqlock.
> > fastopen_cache tried to provide such locking for its values
> > but there is always the risk to access reclaimed entry.
> 
> I basically claim that accidental use of reclaimed entries
> is completely harmless for everything other than fastopen.
> 
> Therefore I do not advocate adding the new overhead and complexity for
> the non-fastopen cases.  It should be a completely free, lockless, and
> synchornization free cache.  If we read crap metrics, so be it, maybe
> the network dynamics changed to the same amount, and we would never
> know the different.  Therefore, it doesn't really matter if we read
> crap values for these measurements.

	OK, it seems I didn't understand you fully in previous
email. So, I can just send patch (or 2) for the tcpm_stamp and
tcp_tw_remember_stamp problems, now or after 2 weeks?

> Thanks.

Regards

--
Julian Anastasov <ja@ssi.bg>

^ permalink raw reply

* Re: [PULL] vhost-net changes for net/3.6
From: Nicholas A. Bellinger @ 2012-07-22 20:34 UTC (permalink / raw)
  To: David Miller
  Cc: mst, netdev, linux-kernel, asias, pbonzini, stefanha, stefanha,
	wuzhy, Greg KH
In-Reply-To: <20120722.121942.2002727319293471163.davem@davemloft.net>

Hi DaveM,

On Sun, 2012-07-22 at 12:19 -0700, David Miller wrote:
> From: "Michael S. Tsirkin" <mst@redhat.com>
> Date: Sun, 22 Jul 2012 01:54:36 +0300
> 
> > The following changes since commit 186e868786f97c8026f0a81400b451ace306b3a4:
> > 
> >   forcedeth: spin_unlock_irq in interrupt handler fix (2012-07-20 16:18:36 -0700)
> > 
> > are available in the git repository at:
> > 
> >   git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost-net-next
> > 
> > for you to fetch changes up to 163049aefdc04323a2d17ec9f2862027b43b0502:
> > 
> >   vhost: make vhost work queue visible (2012-07-22 01:22:23 +0300)
> 
> Pulled, thanks.

So target-pending/for-next-merge will need to be rebased once these bits
hit mainline for the tcm_vhost initial merge commit..

Just curious when your planning on sending out a -rc1 PULL request for
net-next..?

--nab

^ permalink raw reply

* [PATCH] net: Fix references to out-of-scope variables in put_cmsg_compat()
From: Jesper Juhl @ 2012-07-22 21:37 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, linux-kernel

In net/compat.c::put_cmsg_compat() we may assign 'data' the address of
either the 'ctv' or 'cts' local variables inside the 'if
(!COMPAT_USE_64BIT_TIME)' branch.

Those variables go out of scope at the end of the 'if' statement, so
when we use 'data' further down in 'copy_to_user(CMSG_COMPAT_DATA(cm),
data, cmlen - sizeof(struct compat_cmsghdr))' there's no telling what
it may be refering to - not good.

Fix the problem by simply giving 'ctv' and 'cts' function scope.

Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
 net/compat.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/compat.c b/net/compat.c
index 1b96281..74ed1d7 100644
--- a/net/compat.c
+++ b/net/compat.c
@@ -221,6 +221,8 @@ int put_cmsg_compat(struct msghdr *kmsg, int level, int type, int len, void *dat
 {
 	struct compat_cmsghdr __user *cm = (struct compat_cmsghdr __user *) kmsg->msg_control;
 	struct compat_cmsghdr cmhdr;
+	struct compat_timeval ctv;
+	struct compat_timespec cts[3];
 	int cmlen;
 
 	if (cm == NULL || kmsg->msg_controllen < sizeof(*cm)) {
@@ -229,8 +231,6 @@ int put_cmsg_compat(struct msghdr *kmsg, int level, int type, int len, void *dat
 	}
 
 	if (!COMPAT_USE_64BIT_TIME) {
-		struct compat_timeval ctv;
-		struct compat_timespec cts[3];
 		if (level == SOL_SOCKET && type == SCM_TIMESTAMP) {
 			struct timeval *tv = (struct timeval *)data;
 			ctv.tv_sec = tv->tv_sec;
-- 
1.7.11.2


-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

^ permalink raw reply related

* Re: [net-next 00/13][pull request] Intel Wired LAN Driver Updates
From: Jeff Kirsher @ 2012-07-22 21:39 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, gospo, sassmann
In-Reply-To: <20120722.123733.1898891964192153293.davem@davemloft.net>

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

On Sun, 2012-07-22 at 12:37 -0700, David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Sun, 22 Jul 2012 12:24:05 -0700 (PDT)
> 
> > From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > Date: Sat, 21 Jul 2012 16:08:49 -0700
> > 
> >> This series contains updates to ixgbe and ixgbevf.
> >> 
> >> The following are changes since commit
> 186e868786f97c8026f0a81400b451ace306b3a4:
> >>   forcedeth: spin_unlock_irq in interrupt handler fix
> >> and are available in the git repository at:
> >>   git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next
> master
> > 
> > Pulled, thanks Jeff.
> 
> Can you guys actually build test this stuff? 

I did, but it appears I did not have PCI_IOV enabled.  That was my bad,
sorry.

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

^ permalink raw reply

* Re: [PATCH RFC] tcp: use seqlock for all cached tcp_metrics
From: David Miller @ 2012-07-22 21:50 UTC (permalink / raw)
  To: ja; +Cc: netdev
In-Reply-To: <alpine.LFD.2.00.1207222314230.2458@ja.ssi.bg>

From: Julian Anastasov <ja@ssi.bg>
Date: Sun, 22 Jul 2012 23:34:38 +0300 (EEST)

> So, I can just send patch (or 2) for the tcpm_stamp and
> tcp_tw_remember_stamp problems, now or after 2 weeks?

You can send it now.

^ permalink raw reply

* Re: [net-next 00/13][pull request] Intel Wired LAN Driver Updates
From: David Miller @ 2012-07-22 21:53 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: netdev, gospo, sassmann
In-Reply-To: <1342993193.23226.0.camel@jtkirshe-mobl>

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Sun, 22 Jul 2012 14:39:53 -0700

> On Sun, 2012-07-22 at 12:37 -0700, David Miller wrote:
>> From: David Miller <davem@davemloft.net>
>> Date: Sun, 22 Jul 2012 12:24:05 -0700 (PDT)
>> 
>> > From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>> > Date: Sat, 21 Jul 2012 16:08:49 -0700
>> > 
>> >> This series contains updates to ixgbe and ixgbevf.
>> >> 
>> >> The following are changes since commit
>> 186e868786f97c8026f0a81400b451ace306b3a4:
>> >>   forcedeth: spin_unlock_irq in interrupt handler fix
>> >> and are available in the git repository at:
>> >>   git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next
>> master
>> > 
>> > Pulled, thanks Jeff.
>> 
>> Can you guys actually build test this stuff? 
> 
> I did, but it appears I did not have PCI_IOV enabled.  That was my bad,
> sorry.

If you're not doing "allmodconfig" builds, there are by definition
parts you are not testing.  It's the first thing I do with any change
I apply.

^ permalink raw reply

* Re: [PATCH] net: Fix references to out-of-scope variables in put_cmsg_compat()
From: David Miller @ 2012-07-22 21:54 UTC (permalink / raw)
  To: jj; +Cc: netdev, linux-kernel
In-Reply-To: <alpine.LNX.2.00.1207222335200.31033@swampdragon.chaosbits.net>

From: Jesper Juhl <jj@chaosbits.net>
Date: Sun, 22 Jul 2012 23:37:20 +0200 (CEST)

> In net/compat.c::put_cmsg_compat() we may assign 'data' the address of
> either the 'ctv' or 'cts' local variables inside the 'if
> (!COMPAT_USE_64BIT_TIME)' branch.
> 
> Those variables go out of scope at the end of the 'if' statement, so
> when we use 'data' further down in 'copy_to_user(CMSG_COMPAT_DATA(cm),
> data, cmlen - sizeof(struct compat_cmsghdr))' there's no telling what
> it may be refering to - not good.
> 
> Fix the problem by simply giving 'ctv' and 'cts' function scope.
> 
> Signed-off-by: Jesper Juhl <jj@chaosbits.net>

Applied and queued up for -stable, thanks.

^ permalink raw reply

* Re: [PATCH] ppp: add 64 bit stats
From: David Miller @ 2012-07-22 21:54 UTC (permalink / raw)
  To: kgroeneveld; +Cc: netdev
In-Reply-To: <1342988397-3077-1-git-send-email-kgroeneveld@gmail.com>

From: Kevin Groeneveld <kgroeneveld@gmail.com>
Date: Sun, 22 Jul 2012 16:19:56 -0400

> Add 64 bit stats to ppp driver.  The 64 bit stats include tx_bytes,
> rx_bytes, tx_packets and rx_packets.  Other stats are still 32 bit.
> The 64 bit stats can be retrieved via the ndo_get_stats operation.  The
> SIOCGPPPSTATS ioctl is still 32 bit stats only.
> 
> Signed-off-by: Kevin Groeneveld <kgroeneveld@gmail.com>

I don't see this as being very practical nor justified.

^ permalink raw reply

* Re: [PATCH] mlx4: Add support for EEH error recovery
From: David Miller @ 2012-07-23  0:15 UTC (permalink / raw)
  To: ogerlitz; +Cc: klebers, netdev, jackm, yevgenyp, cascardo, brking, shlomop
In-Reply-To: <500BD558.2060803@mellanox.com>

From: Or Gerlitz <ogerlitz@mellanox.com>
Date: Sun, 22 Jul 2012 13:26:32 +0300

> is there anything in the code you added which maybe implicitly
> assumes PPC arch?

He implemented support for a standard PCI API in the kernel, he
happened to test it on a particular platform, and I think that's
the long and short of it.

^ permalink raw reply

* Re: [PATCH] ppp: add 64 bit stats
From: Kevin Groeneveld @ 2012-07-23  0:32 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20120722.145454.943592825984584407.davem@davemloft.net>

On Sun, Jul 22, 2012 at 5:54 PM, David Miller <davem@davemloft.net> wrote:
> From: Kevin Groeneveld <kgroeneveld@gmail.com>
> Date: Sun, 22 Jul 2012 16:19:56 -0400
>
>> Add 64 bit stats to ppp driver.  The 64 bit stats include tx_bytes,
>
> I don't see this as being very practical nor justified.

It is obviously not up to me to decide what is practical or justified
in this case.  I am just always annoyed when I log into my Linksys
router and check the network stats for my PPP interface and have no
idea how many times the transfer stats have rolled over at 4GB.


Kevin

^ permalink raw reply

* Re: net-next compilation failure since 76ff5cc9
From: Neil Horman @ 2012-07-23  0:37 UTC (permalink / raw)
  To: Richard Cochran; +Cc: netdev, Jiri Pirko, David S. Miller
In-Reply-To: <20120722162107.GA23219@netboy.at.omicron.at>

On Sun, Jul 22, 2012 at 06:21:07PM +0200, Richard Cochran wrote:
> Without CONFIG_RPS, net-next fails to compile with
> 
>   CC      net/core/rtnetlink.o
> /linux/net/core/rtnetlink.c: In function ‘rtnl_fill_ifinfo’:
> /linux/net/core/rtnetlink.c:895: error: ‘struct net_device’ has no member named ‘num_rx_queues’
> make[3]: *** [net/core/rtnetlink.o] Error 1
> 
> Looks like this was caused by
> 
> 76ff5cc9  rtnl: allow to specify number of rx and tx queues on device creation
> 
> Not sure how to fix.
> 
> Thanks,
> Richard
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
Its fixed already by 1d69c2b3.  Just pull and you should be good to go.
Neil

^ permalink raw reply

* Re: [PATCH 00/16] Remove the ipv4 routing cache
From: David Miller @ 2012-07-23  0:39 UTC (permalink / raw)
  To: netdev
In-Reply-To: <20120720.142502.1144557295933737451.davem@davemloft.net>


Just FYI, I'm pushing this work out to net-next now.

^ permalink raw reply

* Re: [PATCH] ppp: add 64 bit stats
From: David Miller @ 2012-07-23  0:40 UTC (permalink / raw)
  To: kgroeneveld; +Cc: netdev
In-Reply-To: <CABF+-6XVWpaw6zQe2XNaz4y3oj-KdDm63vbut5jAxs3RWRC6LQ@mail.gmail.com>

From: Kevin Groeneveld <kgroeneveld@gmail.com>
Date: Sun, 22 Jul 2012 20:32:26 -0400

> On Sun, Jul 22, 2012 at 5:54 PM, David Miller <davem@davemloft.net> wrote:
>> From: Kevin Groeneveld <kgroeneveld@gmail.com>
>> Date: Sun, 22 Jul 2012 16:19:56 -0400
>>
>>> Add 64 bit stats to ppp driver.  The 64 bit stats include tx_bytes,
>>
>> I don't see this as being very practical nor justified.
> 
> It is obviously not up to me to decide what is practical or justified
> in this case.  I am just always annoyed when I log into my Linksys
> router and check the network stats for my PPP interface and have no
> idea how many times the transfer stats have rolled over at 4GB.

I guess that's a good point.

^ permalink raw reply

* Re: [PATCH] sctp: Make "Invalid Stream Identifier" ERROR follows SACK when bundling
From: Neil Horman @ 2012-07-23  0:49 UTC (permalink / raw)
  To: xufengzhang.main; +Cc: vyasevich, sri, davem, linux-sctp, netdev, linux-kernel
In-Reply-To: <1342677450-21810-1-git-send-email-xufengzhang.main@gmail.com>

On Thu, Jul 19, 2012 at 01:57:30PM +0800, xufengzhang.main@gmail.com wrote:
> When "Invalid Stream Identifier" ERROR happens after process the
> received DATA chunks, this ERROR chunk is enqueued into outqueue
> before SACK chunk, so when bundling ERROR chunk with SACK chunk,
> the ERROR chunk is always placed first in the packet because of
> the chunk's position in the outqueue.
> This violates sctp specification:
>     RFC 4960 6.5. Stream Identifier and Stream Sequence Number
>     ...The endpoint may bundle the ERROR chunk in the same
>     packet as the SACK as long as the ERROR follows the SACK.
> So we must place SACK first when bundling "Invalid Stream Identifier"
> ERROR and SACK in one packet.
> Although we can do that by enqueue SACK chunk into outqueue before
> ERROR chunk, it will violate the side-effect interpreter processing.
> It's easy to do this job when dequeue chunks from the outqueue,
> by this way, we introduce a flag 'has_isi_err' which indicate
> whether or not the "Invalid Stream Identifier" ERROR happens.
> 
> Signed-off-by: Xufeng Zhang <xufeng.zhang@windriver.com>

Not sure I understand how you came into this error.  If we get an invalid
stream, we issue an SCTP_REPORT_TSN side effect, followed by an SCTP_CMD_REPLY
which sends the error chunk.  The reply goes through
sctp_outq_tail->sctp_outq_chunk->sctp_outq_transmit_chunk->sctp_outq_append_chunk.
That last function checks to see if a sack is already part of the packet, and if
there isn't one, appends one, using the updated tsn map.  So Can you explain in
some more detail how you're getting into this situation?

Thanks!
Neil

^ permalink raw reply

* Re: [PATCH] Fix divide zero crash when xmit with no enabled port
From: ISHIKAWA Mutsumi @ 2012-07-23  0:49 UTC (permalink / raw)
  To: jpirko; +Cc: netdev
In-Reply-To: <20120722135956.GA12129@minipsycho.orion>

>>>>> In <20120722135956.GA12129@minipsycho.orion> 
>>>>>	Jiri Pirko <jpirko@redhat.com> wrote:
>> Sat, Jul 21, 2012 at 10:30:35PM CEST, ishikawa@hanzubon.jp wrote:
>> >
>> >hash calculation in lb_transmit() cause divide zero crash when
>> >xmit on teaming loadbalance mode with no team member port is enabled
>> >(this situation means team->en_port_count = 0). Add check
>> >team->en_port_count is not 0.
>>
>> What kernel are you see the issue one?
>> I believe this is fixed in net-next already:

 Oops, sorry. I've see it on linus's linux-2.6 git tree only.

-- 
ISHIKAWA Mutsumi
 <ishikawa@debian.org>, <ishikawa@hanzubon.jp>, <ishikawa@osdn.jp>

^ permalink raw reply

* many subscribers to this list have been removed
From: David Miller @ 2012-07-23  0:59 UTC (permalink / raw)
  To: netdev


You absolutely cannot bounce back at me, the list owner, simply
because your site is too damn stupid to accept ISO-2022-JP-2
encoded postings.

I refuse to get bombed with bounces from your site just because
your broken email installation refuses to accept postings encoded
in that completely valid character set.

I sort of quietly tolerated this garbage in the past, but it's
a losing game, and people simply need to fix their kit.

If you want to not accept any email encoding in that character
set, that is YOUR business, but once you make such emails
generate a bounce back to the sender (that's me) then it's MY
business.

So people at these sites have two choices 1) turn off the filter or
2) make it quietly drop and not emit a bounce back to the sender.

^ permalink raw reply

* Re: [PATCH V2 resend] ipv6: fix incorrect route 'expires' value passed to userspace
From: Li Wei @ 2012-07-23  1:02 UTC (permalink / raw)
  To: David Laight; +Cc: David Miller, netdev, shemminger
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B6F9C@saturn3.aculab.com>

On 07/20/2012 06:32 PM, David Laight wrote:
>> -	else if (rt->dst.expires - jiffies < INT_MAX)
>> -		expires = rt->dst.expires - jiffies;
>> +	else if ((long)rt->dst.expires - (long)jiffies > INT_MIN
>> +			&& (long)rt->dst.expires - (long)jiffies <
> INT_MAX)
>> +		expires = (long)rt->dst.expires - (long)jiffies;
>>  	else
>> -		expires = INT_MAX;
>> +		expires = time_is_after_jiffies(rt->dst.expires) ?
> INT_MAX : INT_MIN;
> 
> I can't help feeling there is a better way to do this.
> Maybe:
> 	long expires = rt->dst.expires - jiffies;
> 	if (expires != (int)expires)
> 		expires = expires > 0 ? INT_MAX : INT_MIN;
> Although maybe -INT_MAX instead of INT_MIN.
> 
> 	David
> 

Thanks David, your code looks much cleaner and can archieve the same
function except we should use
	long expires = (long)rt->dst.expires - (long)jiffies;
to avoid the wrapping of jiffies.

Thanks, 
Wei

^ permalink raw reply

* Re: [PATCH V2 resend] ipv6: fix incorrect route 'expires' value passed to userspace
From: Li Wei @ 2012-07-23  1:05 UTC (permalink / raw)
  To: David Miller; +Cc: David.Laight, netdev, shemminger
In-Reply-To: <20120720.112241.2111041227435292899.davem@davemloft.net>

On 07/21/2012 02:22 AM, David Miller wrote:
> From: "David Laight" <David.Laight@ACULAB.COM>
> Date: Fri, 20 Jul 2012 11:32:05 +0100
> 
>>> -	else if (rt->dst.expires - jiffies < INT_MAX)
>>> -		expires = rt->dst.expires - jiffies;
>>> +	else if ((long)rt->dst.expires - (long)jiffies > INT_MIN
>>> +			&& (long)rt->dst.expires - (long)jiffies <
>> INT_MAX)
>>> +		expires = (long)rt->dst.expires - (long)jiffies;
>>>  	else
>>> -		expires = INT_MAX;
>>> +		expires = time_is_after_jiffies(rt->dst.expires) ?
>> INT_MAX : INT_MIN;
>>
>> I can't help feeling there is a better way to do this.
>> Maybe:
>> 	long expires = rt->dst.expires - jiffies;
>> 	if (expires != (int)expires)
>> 		expires = expires > 0 ? INT_MAX : INT_MIN;
>> Although maybe -INT_MAX instead of INT_MIN.
> 
> This patch also does not apply at all to net-next, so needs to be
> redone regardless.

I'll redone this patch base on 'net-next'.

Thanks

> 
> 

^ permalink raw reply

* Re: [PATCH] net, cgroup: Fix boot failure due to iteration of uninitialized list
From: Gao feng @ 2012-07-23  1:15 UTC (permalink / raw)
  To: Srivatsa S. Bhat, eric.dumazet, davem
  Cc: nhorman, linux-kernel, netdev, mark.d.rustad, john.r.fastabend,
	lizefan
In-Reply-To: <20120719162532.23505.85946.stgit@srivatsabhat.in.ibm.com>

于 2012年07月20日 00:27, Srivatsa S. Bhat 写道:
> After commit ef209f15 (net: cgroup: fix access the unallocated memory in
> netprio cgroup), boot fails with the following NULL pointer dereference:
> 
> Initializing cgroup subsys devices
> Initializing cgroup subsys freezer
> Initializing cgroup subsys net_cls
> Initializing cgroup subsys blkio
> Initializing cgroup subsys perf_event
> Initializing cgroup subsys net_prio
> BUG: unable to handle kernel NULL pointer dereference at 0000000000000698
> IP: [<ffffffff8145e8d6>] cgrp_create+0xf6/0x190
> PGD 0
> Oops: 0000 [#1] SMP
> CPU 0
> Modules linked in:
> 
> Pid: 0, comm: swapper/0 Not tainted 3.5.0-rc7-mandeep #1 IBM IBM System x -[7870C4Q]-/68Y8033
> RIP: 0010:[<ffffffff8145e8d6>]  [<ffffffff8145e8d6>] cgrp_create+0xf6/0x190
> RSP: 0000:ffffffff81a01ea8  EFLAGS: 00010213
> RAX: 0000000000000000 RBX: ffffffffffffff10 RCX: 0000000000000000
> RDX: 0000000000000000 RSI: 0000000000000246 RDI: ffffffff81aa70a0
> RBP: ffffffff81a01ed8 R08: 0000000000000000 R09: 0000000000000000
> R10: ffff8808ff8641c0 R11: 6e697a696c616974 R12: 0000000000000001
> R13: ffff8808ff8641c0 R14: 0000000000000000 R15: 0000000000093970
> FS:  0000000000000000(0000) GS:ffff8808ffc00000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> CR2: 0000000000000698 CR3: 0000000001a0b000 CR4: 00000000000006b0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> Process swapper/0 (pid: 0, threadinfo ffffffff81a00000, task ffffffff81a13420)
> Stack:
>  ffffffff81a01eb8 ffffffff818060ff ffffffff81d75ec8 ffffffff81aa8960
>  ffffffff81aa8960 ffffffff81b4c2c0 ffffffff81a01ef8 ffffffff81b1cb78
>  0000000000000018 0000000000000048 ffffffff81a01f18 ffffffff81b1ce13
> Call Trace:
>  [<ffffffff81b1cb78>] cgroup_init_subsys+0x83/0x169
>  [<ffffffff81b1ce13>] cgroup_init+0x36/0x119
>  [<ffffffff81affef7>] start_kernel+0x3ba/0x3ef
>  [<ffffffff81aff95b>] ? kernel_init+0x27b/0x27b
>  [<ffffffff81aff356>] x86_64_start_reservations+0x131/0x136
>  [<ffffffff81aff45e>] x86_64_start_kernel+0x103/0x112
> Code: 01 48 3d f8 e1 ec 81 48 8d 98 10 ff ff ff 75 1b eb 73 0f 1f 00 48 8b 83 f0 00 00 00 48 3d f8 e1 ec 81 48 8d 98 10 ff ff ff 74 5a <48> 8b 83 88 07 00 00 48 85 c0 74 de 44 3b 60 10 76 d8 44 89 e6
> RIP  [<ffffffff8145e8d6>] cgrp_create+0xf6/0x190
>  RSP <ffffffff81a01ea8>
> CR2: 0000000000000698
> ---[ end trace a7919e7f17c0a725 ]---
> Kernel panic - not syncing: Attempted to kill the idle task!
> 
> The code corresponds to:
> 
> update_netdev_tables():
>         for_each_netdev(&init_net, dev) {
>                 map = rtnl_dereference(dev->priomap);  <---- HERE
> 
> 
> The list head is initialized in netdev_init(), which is called much
> later than cgrp_create(). So the problem is that we are calling
> update_netdev_tables() way too early (in cgrp_create()), which will
> end up traversing the not-yet-circular linked list. So at some point,
> the dev pointer will become NULL and hence dev->priomap becomes an
> invalid access.
> 
> To fix this, just remove the update_netdev_tables() function entirely,
> since it appears that write_update_netdev_table() will handle things
> just fine.

The reason I add update_netdev_tables in cgrp_create is to avoid additional
bound checkings when we accessing the dev->priomap.priomap.

Eric,can we revert this commit 91c68ce2b26319248a32d7baa1226f819d283758 now?
I think it's safe enough to access priomap without bound check.

Thanks

^ permalink raw reply

* Re: [PATCH] sctp: Make "Invalid Stream Identifier" ERROR follows SACK when bundling
From: xufeng zhang @ 2012-07-23  2:30 UTC (permalink / raw)
  To: Neil Horman
  Cc: xufengzhang.main, vyasevich, sri, davem, linux-sctp, netdev,
	linux-kernel
In-Reply-To: <20120723004932.GB8040@neilslaptop.think-freely.org>

On 07/23/2012 08:49 AM, Neil Horman wrote:
>
> Not sure I understand how you came into this error.  If we get an invalid
> stream, we issue an SCTP_REPORT_TSN side effect, followed by an SCTP_CMD_REPLY
> which sends the error chunk.  The reply goes through
> sctp_outq_tail->sctp_outq_chunk->sctp_outq_transmit_chunk->sctp_outq_append_chunk.
> That last function checks to see if a sack is already part of the packet, and if
> there isn't one, appends one, using the updated tsn map.
Yes, you are right, but consider the invalid stream identifier's DATA 
chunk is the first
DATA chunk in the association which will need SACK immediately.
Here is what I thought of the scenario:
     sctp_sf_eat_data_6_2()
         -->sctp_eat_data()
             -->sctp_make_op_error()
             -->sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(err))
             -->sctp_outq_tail()          /* First enqueue ERROR chunk */
         -->sctp_add_cmd_sf(commands, SCTP_CMD_GEN_SACK, SCTP_FORCE())
             -->sctp_gen_sack()
                 -->sctp_make_sack()
                 -->sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, 
SCTP_CHUNK(sack))
                 -->sctp_outq_tail()          /* Then enqueue SACK chunk */

So SACK chunk is enqueued after ERROR chunk.
> So Can you explain in
> some more detail how you're getting into this situation?
>    
Actually it's triggered by a customer's test case, but we can also 
reproduce this problem
easily by explicitly contaminating the sctp stack:
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -701,7 +701,7 @@ struct sctp_chunk *sctp_make_datafrag_empty(struct 
sctp_association *asoc,
          * creating the chunk.
          */
         dp.tsn = 0;
-       dp.stream = htons(sinfo->sinfo_stream);
+       dp.stream = htons(sinfo->sinfo_stream) + 10;
         dp.ppid   = sinfo->sinfo_ppid;

         /* Set the flags for an unordered send.  */


Then run sctp_darn application and capture the sctp packet at the same time.



Thanks,
Xufeng Zhang
> Thanks!
> Neil
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>
>    

^ permalink raw reply

* [PATCH net] rds: set correct msg_namelen
From: Weiping Pan @ 2012-07-23  2:37 UTC (permalink / raw)
  To: netdev; +Cc: linux-kernel

Jay Fenlason (fenlason@redhat.com) found a bug,
that recvfrom() on an RDS socket can return the contents of random kernel
memory to userspace if it was called with a address length larger than
sizeof(struct sockaddr_in).
rds_recvmsg() also fails to set the addr_len paramater properly before
returning, but that's just a bug.
There are also a number of cases wher recvfrom() can return an entirely bogus
address. Anything in rds_recvmsg() that returns a non-negative value but does
not go through the "sin = (struct sockaddr_in *)msg->msg_name;" code path
at the end of the while(1) loop will return up to 128 bytes of kernel memory
to userspace.

And I write two test programs to reproduce this bug, you will see that in
rds_server, fromAddr will be overwritten and the following sock_fd will be
destroyed.
Yes, it is the programmer's fault to set msg_namelen incorrectly, but it is
better to make the kernel copy the real length of address to user space in
such case.

How to run the test programs ?
I test them on 32bit x86 system, 3.5.0-rc7.

1 compile
gcc -o rds_client rds_client.c
gcc -o rds_server rds_server.c

2 run ./rds_server on one console

3 run ./rds_client on another console

4 you will see something like:
server is waiting to receive data...
old socket fd=3
server received data from client:data from client
msg.msg_namelen=32
new socket fd=-1067277685
sendmsg()
: Bad file descriptor

/***************** rds_client.c ********************/

int main(void)
{
	int sock_fd;
	struct sockaddr_in serverAddr;
	struct sockaddr_in toAddr;
	char recvBuffer[128] = "data from client";
	struct msghdr msg;
	struct iovec iov;

	sock_fd = socket(AF_RDS, SOCK_SEQPACKET, 0);
	if (sock_fd < 0) {
		perror("create socket error\n");
		exit(1);
	}

	memset(&serverAddr, 0, sizeof(serverAddr));
	serverAddr.sin_family = AF_INET;
	serverAddr.sin_addr.s_addr = inet_addr("127.0.0.1");
	serverAddr.sin_port = htons(4001);

	if (bind(sock_fd, (struct sockaddr*)&serverAddr, sizeof(serverAddr)) < 0) {
		perror("bind() error\n");
		close(sock_fd);
		exit(1);
	}

	memset(&toAddr, 0, sizeof(toAddr));
	toAddr.sin_family = AF_INET;
	toAddr.sin_addr.s_addr = inet_addr("127.0.0.1");
	toAddr.sin_port = htons(4000);
	msg.msg_name = &toAddr;
	msg.msg_namelen = sizeof(toAddr);
	msg.msg_iov = &iov;
	msg.msg_iovlen = 1;
	msg.msg_iov->iov_base = recvBuffer;
	msg.msg_iov->iov_len = strlen(recvBuffer) + 1;
	msg.msg_control = 0;
	msg.msg_controllen = 0;
	msg.msg_flags = 0;

	if (sendmsg(sock_fd, &msg, 0) == -1) {
		perror("sendto() error\n");
		close(sock_fd);
		exit(1);
	}

	printf("client send data:%s\n", recvBuffer);

	memset(recvBuffer, '\0', 128);

	msg.msg_name = &toAddr;
	msg.msg_namelen = sizeof(toAddr);
	msg.msg_iov = &iov;
	msg.msg_iovlen = 1;
	msg.msg_iov->iov_base = recvBuffer;
	msg.msg_iov->iov_len = 128;
	msg.msg_control = 0;
	msg.msg_controllen = 0;
	msg.msg_flags = 0;
	if (recvmsg(sock_fd, &msg, 0) == -1) {
		perror("recvmsg() error\n");
		close(sock_fd);
		exit(1);
	}

	printf("receive data from server:%s\n", recvBuffer);

	close(sock_fd);

	return 0;
}

/***************** rds_server.c ********************/

int main(void)
{
	struct sockaddr_in fromAddr;
	int sock_fd;
	struct sockaddr_in serverAddr;
	unsigned int addrLen;
	char recvBuffer[128];
	struct msghdr msg;
	struct iovec iov;

	sock_fd = socket(AF_RDS, SOCK_SEQPACKET, 0);
	if(sock_fd < 0) {
		perror("create socket error\n");
		exit(0);
	}

	memset(&serverAddr, 0, sizeof(serverAddr));
	serverAddr.sin_family = AF_INET;
	serverAddr.sin_addr.s_addr = inet_addr("127.0.0.1");
	serverAddr.sin_port = htons(4000);
	if (bind(sock_fd, (struct sockaddr*)&serverAddr, sizeof(serverAddr)) < 0) {
		perror("bind error\n");
		close(sock_fd);
		exit(1);
	}

	printf("server is waiting to receive data...\n");
	msg.msg_name = &fromAddr;

	/*
	 * I add 16 to sizeof(fromAddr), ie 32,
	 * and pay attention to the definition of fromAddr,
	 * recvmsg() will overwrite sock_fd,
	 * since kernel will copy 32 bytes to userspace.
	 *
	 * If you just use sizeof(fromAddr), it works fine.
	 * */
	msg.msg_namelen = sizeof(fromAddr) + 16;
	/* msg.msg_namelen = sizeof(fromAddr); */
	msg.msg_iov = &iov;
	msg.msg_iovlen = 1;
	msg.msg_iov->iov_base = recvBuffer;
	msg.msg_iov->iov_len = 128;
	msg.msg_control = 0;
	msg.msg_controllen = 0;
	msg.msg_flags = 0;

	while (1) {
		printf("old socket fd=%d\n", sock_fd);
		if (recvmsg(sock_fd, &msg, 0) == -1) {
			perror("recvmsg() error\n");
			close(sock_fd);
			exit(1);
		}
		printf("server received data from client:%s\n", recvBuffer);
		printf("msg.msg_namelen=%d\n", msg.msg_namelen);
		printf("new socket fd=%d\n", sock_fd);
		strcat(recvBuffer, "--data from server");
		if (sendmsg(sock_fd, &msg, 0) == -1) {
			perror("sendmsg()\n");
			close(sock_fd);
			exit(1);
		}
	}

	close(sock_fd);
	return 0;
}

Signed-off-by: Weiping Pan <wpan@redhat.com>
---
 net/rds/recv.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/net/rds/recv.c b/net/rds/recv.c
index 5c6e9f1..9f0f17c 100644
--- a/net/rds/recv.c
+++ b/net/rds/recv.c
@@ -410,6 +410,8 @@ int rds_recvmsg(struct kiocb *iocb, struct socket *sock, struct msghdr *msg,
 
 	rdsdebug("size %zu flags 0x%x timeo %ld\n", size, msg_flags, timeo);
 
+	msg->msg_namelen = 0;
+
 	if (msg_flags & MSG_OOB)
 		goto out;
 
@@ -485,6 +487,7 @@ int rds_recvmsg(struct kiocb *iocb, struct socket *sock, struct msghdr *msg,
 			sin->sin_port = inc->i_hdr.h_sport;
 			sin->sin_addr.s_addr = inc->i_saddr;
 			memset(sin->sin_zero, 0, sizeof(sin->sin_zero));
+			msg->msg_namelen = sizeof(*sin);
 		}
 		break;
 	}
-- 
1.7.4

^ permalink raw reply related

* Re: [PATCH] sctp: Make "Invalid Stream Identifier" ERROR follows SACK when bundling
From: xufeng zhang @ 2012-07-23  5:16 UTC (permalink / raw)
  To: vyasevich, sri, davem; +Cc: xufengzhang.main, linux-sctp, netdev, linux-kernel
In-Reply-To: <1342677450-21810-1-git-send-email-xufengzhang.main@gmail.com>

On 07/19/2012 01:57 PM, xufengzhang.main@gmail.com wrote:
> When "Invalid Stream Identifier" ERROR happens after process the
> received DATA chunks, this ERROR chunk is enqueued into outqueue
> before SACK chunk, so when bundling ERROR chunk with SACK chunk,
> the ERROR chunk is always placed first in the packet because of
> the chunk's position in the outqueue.
> This violates sctp specification:
>      RFC 4960 6.5. Stream Identifier and Stream Sequence Number
>      ...The endpoint may bundle the ERROR chunk in the same
>      packet as the SACK as long as the ERROR follows the SACK.
> So we must place SACK first when bundling "Invalid Stream Identifier"
> ERROR and SACK in one packet.
> Although we can do that by enqueue SACK chunk into outqueue before
> ERROR chunk, it will violate the side-effect interpreter processing.
> It's easy to do this job when dequeue chunks from the outqueue,
> by this way, we introduce a flag 'has_isi_err' which indicate
> whether or not the "Invalid Stream Identifier" ERROR happens.
>
> Signed-off-by: Xufeng Zhang<xufeng.zhang@windriver.com>
> ---
>   include/net/sctp/structs.h |    2 ++
>   net/sctp/output.c          |   26 ++++++++++++++++++++++++++
>   2 files changed, 28 insertions(+), 0 deletions(-)
>
> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
> index 88949a9..5adf4de 100644
> --- a/include/net/sctp/structs.h
> +++ b/include/net/sctp/structs.h
> @@ -842,6 +842,8 @@ struct sctp_packet {
>   	    has_sack:1,		/* This packet contains a SACK chunk. */
>   	    has_auth:1,		/* This packet contains an AUTH chunk */
>   	    has_data:1,		/* This packet contains at least 1 DATA chunk */
> +	    has_isi_err:1,	/* This packet contains a "Invalid Stream
> +				 * Identifier" ERROR chunk */
>   	    ipfragok:1,		/* So let ip fragment this packet */
>   	    malloced:1;		/* Is it malloced? */
>   };
> diff --git a/net/sctp/output.c b/net/sctp/output.c
> index 817174e..77fb1ae 100644
> --- a/net/sctp/output.c
> +++ b/net/sctp/output.c
> @@ -79,6 +79,7 @@ static void sctp_packet_reset(struct sctp_packet *packet)
>   	packet->has_sack = 0;
>   	packet->has_data = 0;
>   	packet->has_auth = 0;
> +	packet->has_isi_err = 0;
>   	packet->ipfragok = 0;
>   	packet->auth = NULL;
>   }
> @@ -267,6 +268,7 @@ static sctp_xmit_t sctp_packet_bundle_sack(struct sctp_packet *pkt,
>   sctp_xmit_t sctp_packet_append_chunk(struct sctp_packet *packet,
>   				     struct sctp_chunk *chunk)
>   {
> +	struct sctp_chunk *lchunk;
>   	sctp_xmit_t retval = SCTP_XMIT_OK;
>   	__u16 chunk_len = WORD_ROUND(ntohs(chunk->chunk_hdr->length));
>
> @@ -316,7 +318,31 @@ sctp_xmit_t sctp_packet_append_chunk(struct sctp_packet *packet,
>   		packet->has_cookie_echo = 1;
>   		break;
>
> +	    case SCTP_CID_ERROR:
> +		if (chunk->subh.err_hdr->cause&  SCTP_ERROR_INV_STRM)
> +			packet->has_isi_err = 1;
> +		break;
> +
>   	    case SCTP_CID_SACK:
> +		/* RFC 4960
> +		 * 6.5 Stream Identifier and Stream Sequence Number
> +		 * The endpoint may bundle the ERROR chunk in the same
> +		 * packet as the SACK as long as the ERROR follows the SACK.
> +		 */
> +		if (packet->has_isi_err) {
> +			if (list_is_singular(&packet->chunk_list))
> +				list_add(&chunk->list,&packet->chunk_list);
> +			else {
> +				lchunk = list_first_entry(&packet->chunk_list,
> +						struct sctp_chunk, list);
> +				list_add(&chunk->list,&lchunk->list);
> +			}
>    
And I should clarify the above judgment code.
AFAIK, there should be two cases for the bundling when invalid stream 
identifier error happens:
1). COOKIE_ACK ERROR SACK
2). ERROR SACK
So I need to deal with the two cases differently.


Thanks,
Xufeng Zhang
> +			packet->size += chunk_len;
> +			chunk->transport = packet->transport;
> +			packet->has_sack = 1;
> +			goto finish;
> +		}
> +
>   		packet->has_sack = 1;
>   		break;
>
>    

^ 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