Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 16/51] DMA-API: ppc: vio.c: replace dma_set_mask()+dma_set_coherent_mask() with new helper
From: Cedric Le Goater @ 2013-11-15 16:16 UTC (permalink / raw)
  To: Russell King
  Cc: alsa-devel, b43-dev, devel, devicetree, dri-devel, e1000-devel,
	linux-arm-kernel, linux-crypto, linux-doc, linux-fbdev, linux-ide,
	linux-media, linux-mmc, linux-nvme, linux-omap, linuxppc-dev,
	linux-samsung-soc, linux-scsi, linux-tegra, linux-usb,
	linux-wireless, netdev, Solarflare linux maintainers,
	uclinux-dist-devel, Paul Mackerras, Benjamin Herrenschmidt
In-Reply-To: <E1VMly8-0007gy-Ru@rmk-PC.arm.linux.org.uk>

Hi, 

On 09/19/2013 11:41 PM, Russell King wrote:
> Replace the following sequence:
> 
> 	dma_set_mask(dev, mask);
> 	dma_set_coherent_mask(dev, mask);
> 
> with a call to the new helper dma_set_mask_and_coherent().
> 
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
>  arch/powerpc/kernel/vio.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/vio.c b/arch/powerpc/kernel/vio.c
> index 78a3506..96b6c97 100644
> --- a/arch/powerpc/kernel/vio.c
> +++ b/arch/powerpc/kernel/vio.c
> @@ -1413,8 +1413,7 @@ struct vio_dev *vio_register_device_node(struct device_node *of_node)
> 
>  		/* needed to ensure proper operation of coherent allocations
>  		 * later, in case driver doesn't set it explicitly */
> -		dma_set_mask(&viodev->dev, DMA_BIT_MASK(64));
> -		dma_set_coherent_mask(&viodev->dev, DMA_BIT_MASK(64));
> +		dma_set_mask_and_coherent(&viodev->dev, DMA_BIT_MASK(64));
>  	}
> 
>  	/* register with generic device framework */
> 

The new helper routine dma_set_mask_and_coherent() breaks the 
initialization of the pseries vio devices which do not have an 
initial dev->dma_mask. I think we need to use dma_coerce_mask_and_coherent()
instead.

Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
---
 arch/powerpc/kernel/vio.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/vio.c b/arch/powerpc/kernel/vio.c
index e7d0c88f..76a6482 100644
--- a/arch/powerpc/kernel/vio.c
+++ b/arch/powerpc/kernel/vio.c
@@ -1419,7 +1419,7 @@ struct vio_dev *vio_register_device_node(struct device_node *of_node)
 
 		/* needed to ensure proper operation of coherent allocations
 		 * later, in case driver doesn't set it explicitly */
-		dma_set_mask_and_coherent(&viodev->dev, DMA_BIT_MASK(64));
+		dma_coerce_mask_and_coherent(&viodev->dev, DMA_BIT_MASK(64));
 	}
 
 	/* register with generic device framework */
-- 
1.7.10.4

^ permalink raw reply related

* RE: [PATCH v2] WAN: Adding support for Infineon PEF2256 E1 chipset (FALC56)
From: thomas.langer @ 2013-11-15 15:17 UTC (permalink / raw)
  To: christophe.leroy
  Cc: devicetree, linux-doc, linux-kernel, netdev,
	jerome.chantelauze.ext
In-Reply-To: <201311011501.rA1F1suj007279@localhost.localdomain>

Hello Christophe,

Christophe Leroy wrote on 2013-11-01:

> The patch adds WAN support for Infineon FALC56 - PEF2256 E1 Chipset.

can you please replace "Infineon" with "Lantiq"?
The former "wire-line" business-unit was transferred into an own company some years ago.

Maybe provide a comment in the description or binding document, to avoid more confusion about this.

> 
> Signed-off-by: Jerome Chantelauze <jerome.chantelauze.ext@c-s.fr>
> Acked-by: Christophe Leroy <christophe.leroy@c-s.fr>
> 

Thanks,
Thomas

---
There are two hard things in computer science: cache invalidation, naming things, and off-by-one errors.
---

^ permalink raw reply

* [RFC PATCH net-next] net: reorder struct netns_ct for better cache-line usage
From: Jesper Dangaard Brouer @ 2013-11-15 14:57 UTC (permalink / raw)
  To: Pablo Neira Ayuso, Eric Dumazet
  Cc: netdev, netfilter-devel, Jesper Dangaard Brouer

Reorder struct netns_ct so that atomic_t "count" changes don't
slowdown users of read mostly fields.

This is based on Eric Dumazet's proposed patch:
 "netfilter: conntrack: remove the central spinlock"
 http://thread.gmane.org/gmane.linux.network/268758/focus=47306

The tricky part of cache-aligning this structure, that it is getting
inlined in struct net (include/net/net_namespace.h), thus changes to
other netns_xxx structures affects our alignment.

Eric's original patch contained an ambiguity on 32-bit regarding
alignment in struct net.  This patch also takes 32-bit into account,
and in case of changed (struct net) alignment sysctl_xxx entries have
been ordered according to how often they are accessed.

Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
Reviewed-by: Jiri Benc <jbenc@redhat.com>
---
Yes, I know it's not the time to submit this patch, but
I just want to let Eric know that I'm planning on extracting
part of his proposed patch, which have been stalled for months.
I'll resubmit once net-next/nf-next opens for changes again.


 include/net/netns/conntrack.h |   33 +++++++++++++++++----------------
 1 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/include/net/netns/conntrack.h b/include/net/netns/conntrack.h
index c9c0c53..fbcc7fa 100644
--- a/include/net/netns/conntrack.h
+++ b/include/net/netns/conntrack.h
@@ -65,6 +65,23 @@ struct nf_ip_net {
 struct netns_ct {
 	atomic_t		count;
 	unsigned int		expect_count;
+#ifdef CONFIG_SYSCTL
+	struct ctl_table_header	*sysctl_header;
+	struct ctl_table_header	*acct_sysctl_header;
+	struct ctl_table_header	*tstamp_sysctl_header;
+	struct ctl_table_header	*event_sysctl_header;
+	struct ctl_table_header	*helper_sysctl_header;
+#endif
+	char			*slabname;
+	unsigned int		sysctl_log_invalid; /* Log invalid packets */
+	unsigned int		sysctl_events_retry_timeout;
+	int			sysctl_events;
+	int			sysctl_acct;
+	int			sysctl_auto_assign_helper;
+	bool			auto_assign_helper_warned;
+	int			sysctl_tstamp;
+	int			sysctl_checksum;
+
 	unsigned int		htable_size;
 	struct kmem_cache	*nf_conntrack_cachep;
 	struct hlist_nulls_head	*hash;
@@ -75,14 +92,6 @@ struct netns_ct {
 	struct ip_conntrack_stat __percpu *stat;
 	struct nf_ct_event_notifier __rcu *nf_conntrack_event_cb;
 	struct nf_exp_event_notifier __rcu *nf_expect_event_cb;
-	int			sysctl_events;
-	unsigned int		sysctl_events_retry_timeout;
-	int			sysctl_acct;
-	int			sysctl_tstamp;
-	int			sysctl_checksum;
-	unsigned int		sysctl_log_invalid; /* Log invalid packets */
-	int			sysctl_auto_assign_helper;
-	bool			auto_assign_helper_warned;
 	struct nf_ip_net	nf_ct_proto;
 #if defined(CONFIG_NF_CONNTRACK_LABELS)
 	unsigned int		labels_used;
@@ -92,13 +101,5 @@ struct netns_ct {
 	struct hlist_head	*nat_bysource;
 	unsigned int		nat_htable_size;
 #endif
-#ifdef CONFIG_SYSCTL
-	struct ctl_table_header	*sysctl_header;
-	struct ctl_table_header	*acct_sysctl_header;
-	struct ctl_table_header	*tstamp_sysctl_header;
-	struct ctl_table_header	*event_sysctl_header;
-	struct ctl_table_header	*helper_sysctl_header;
-#endif
-	char			*slabname;
 };
 #endif


^ permalink raw reply related

* Re: [PATCH] net: sctp: recover a tranport when an ack comes
From: Neil Horman @ 2013-11-15 14:56 UTC (permalink / raw)
  To: Vlad Yasevich
  Cc: Chang Xiangzhong, davem, linux-sctp, netdev, linux-kernel, dreibh,
	ernstgr
In-Reply-To: <5286291A.7060507@gmail.com>

On Fri, Nov 15, 2013 at 09:00:58AM -0500, Vlad Yasevich wrote:
> On 11/15/2013 07:30 AM, Neil Horman wrote:
> >On Thu, Nov 14, 2013 at 09:34:55PM -0500, Vlad Yasevich wrote:
> >>On 11/14/2013 03:40 PM, Chang Xiangzhong wrote:
> >>>Expected Behavior:
> >>>When hearing an ack from a tranport/path, set its state to normal/on if it's
> >>>in abnormal(__partial_failure__ or inactive) state.
> >>>
> >>>state machine of tranport->state
> >>>Whenever a T3_RTX timer expires, then transport->error_count++.
> >>>When (association->pf_retrans < transport->error_count < tranport->pathmaxrtx)
> >>>	transport->state = SCTP_PF //partial failure
> >>>
> >>>When a heartbeat-ack comes or conventional ack acknowledged its availability,
> >>>	transport->state = SCTP_ON
> >>>
> >>>Signed-off-by: Chang Xiangzhong <changxiangzhong@gmail.com>
> >>>Fixes: 5aa93bcf66f ("sctp: Implement quick failover draft from tsvwg")
> >>
> >>I don't think this is right.  The spec states:
> >>  8.  ACKs for retransmissions do not transition a PF destination back
> >>        to Active state, since a sender cannot disambiguate whether the
> >>        ack was for the original transmission or the retransmission(s).
> >>
> >>
> >>Now, the proper way to this would would be modify
> >>sctp_assoc_control_transport() to transition the transport state to
> >>ACTIVE if it was PF transport that was chosen to send data.
> >>
> >>-vlad
> >>
> >I agree, this patch doesn't agree with the spec, the only time we transition
> >from PF to ACTIVE should be on receipt of ack of new data.
> 
> You mean HB ACK, right?  The 02 spec that see on the ietf site doesn't
> mention anything about transition on SACKs.  Also, there is no way to
> tell right now if the ack is for new or retransmitted data.  We could
> mark chunks that are retransmitted though.
> 
Yes, sorry I wasn't clear, I was speaknig about HB Acks.

> > I'm not even sure if
> >we should allow PF transports to be selected to send new data.  Currently a
> >potentially failed transport will get ignored when specified, and the stack will
> >use the active path in its place.  Only if all transports are PF will a PF
> >transport be chosen.
> 
> Not even that :(.  If all transports are PF, we are going to camp on
> the primary path instead of choosing a PF transport with the lowest
> error count.
> 
Yes, we don't do the smallest error count check, though I have to wonder if
thats really worthwhile.  If all your transports are PF, you're a step away from
a connection reset anyway.
Neil

> -vlad
> 
> >Neil
> >
> >>>---
> >>>  net/sctp/outqueue.c |    1 +
> >>>  1 file changed, 1 insertion(+)
> >>>
> >>>diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
> >>>index 94df758..2557fa5 100644
> >>>--- a/net/sctp/outqueue.c
> >>>+++ b/net/sctp/outqueue.c
> >>>@@ -1517,6 +1517,7 @@ static void sctp_check_transmitted(struct sctp_outq *q,
> >>>  			 * active if it is not so marked.
> >>>  			 */
> >>>  			if ((transport->state == SCTP_INACTIVE ||
> >>>+			     transport->state == SCTP_PF ||
> >>>  			     transport->state == SCTP_UNCONFIRMED) &&
> >>>  			    sctp_cmp_addr_exact(&transport->ipaddr, saddr)) {
> >>>  				sctp_assoc_control_transport(
> >>>
> >>
> >>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply

* Re: [PATCH 1/4] wl1251: split wl251 platform data to a separate structure
From: Sebastian Reichel @ 2013-11-15 14:55 UTC (permalink / raw)
  To: John W. Linville
  Cc: Tony Lindgren, Luciano Coelho, Rob Herring, Pawel Moll,
	Mark Rutland, Stephen Warren, Ian Campbell, Rob Landley,
	Russell King, Felipe Balbi, Sachin Kamat, Greg Kroah-Hartman,
	Bill Pemberton, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, Luciano Coelho
In-Reply-To: <20131115143254.GA7079-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>

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

On Fri, Nov 15, 2013 at 09:32:55AM -0500, John W. Linville wrote:
> On Thu, Nov 14, 2013 at 03:22:18PM -0800, Tony Lindgren wrote:
> > * Sebastian Reichel <sre-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org> [131114 15:04]:
> > > On Thu, Nov 14, 2013 at 10:51:33AM -0800, Tony Lindgren wrote:
> > > > [...]
> > > >
> > > > If this is not going into v3.13, these will cause conflicts
> > > > with the mach-omap2/board-*.c files for v3.14.
> > > > 
> > > > So it might be best to do a minimal header patch first that
> > > > can be merged in by both linux-omap and wireless trees.
> > > 
> > > I guess this patch is pretty minimal. It also seems to be acked by
> > > the involved Maintainers, so maybe just merge Patch 1 without the
> > > other patches?
> > > 
> > > This does not solve the problem with the struct modification from
> > > the second patch, but I guess it's the more intrusive patch.
> > 
> > Once at least the first two patches are ready, how about I queue
> > them after -rc1 and set up an immutable branch that can be merged
> > in by linux-omap tree and the wireless tree?
> 
> That sounds reasonable to me.

So what changes do you request for those patches? I will try to
update them as soon as possible.

-- Sebastian

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

^ permalink raw reply

* Re: When a TCP segment is split up (to be sent through a TUN device with a small MTU) who should recalculate the checksum?
From: John Hughes @ 2013-11-15 14:55 UTC (permalink / raw)
  To: vyasevic; +Cc: John Hughes, Vlad Yasevich, netdev
In-Reply-To: <528632AA.2090400@redhat.com>

On 15/11/13 15:41, Vlad Yasevich wrote:
> On 11/15/2013 09:31 AM, John Hughes wrote:
>> On 15/11/13 15:20, Vlad Yasevich wrote:
>>>
>>> Can you check to see if you have the following patch in your kernel
>>> commit: 1cdbcb7957cf9e5f841dbcde9b38fd18a804208b

> Give it a try.  It has helped me in similar situations.  If it helps,
> we can get it into the stable tree.

Ok, I'll make myself a kernel with that patch.

Should be able to try it over the weekend.

-- 
John.

^ permalink raw reply

* Re: [PATCH 0/9] genetlink: reduce ops size and complexity (v2)
From: Jamal Hadi Salim @ 2013-11-15 14:45 UTC (permalink / raw)
  To: Johannes Berg, David Miller
  Cc: netdev, linux-wimax, bsingharora, netfilter-devel,
	alex.bluesman.smirnov, dbaryshkov
In-Reply-To: <1384521794.14295.5.camel@jlt4.sipsolutions.net>


Are we killing APIs that are already used?
At least that patches that Dave sucked in do. Out of tree
users dont matter?

cheers,
jamal

On 11/15/13 08:23, Johannes Berg wrote:
> On Fri, 2013-11-15 at 14:18 +0100, Johannes Berg wrote:
>
>> I've been eyeing the multicast groups as well
>
> Of course, there are also *much* fewer mcast groups, so the saving isn't
> nearly as big. But we still have some oddball code to register them all,
> basically
>
> 	err = register_mc_group();
> 	if (err)
> 		goto unregister_family;
>
> (since we have to register the family first, afaict)
>
> sometimes a few of those back to back, and it'd be nicer on the users if
> that just went away and was
>
> 	family.mcast_groups = my_groups;
> 	family.n_mcast_groups = ARRAY_SIZE(my_groups);
> 	family.ops = my_ops;
> 	family.n_ops = ARRAY_SIZE(my_ops);
> 	return genl_register_family(&family);
>
> since nobody uses them dynamically anyway, afaict.
>
> johannes
>
> --
> 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
>


^ permalink raw reply

* Re: When a TCP segment is split up (to be sent through a TUN device with a small MTU) who should recalculate the checksum?
From: Vlad Yasevich @ 2013-11-15 14:41 UTC (permalink / raw)
  To: John Hughes, Vlad Yasevich; +Cc: netdev
In-Reply-To: <5286305F.5060903@atlantech.com>

On 11/15/2013 09:31 AM, John Hughes wrote:
> On 15/11/13 15:20, Vlad Yasevich wrote:
>>
>> Can you check to see if you have the following patch in your kernel
>> commit: 1cdbcb7957cf9e5f841dbcde9b38fd18a804208b
>> Author: Simon Horman <horms@verge.net.au>
>> Date:   Sun May 19 15:46:49 2013 +0000
>>
>>     net: Loosen constraints for recalculating checksum in skb_segment()
>>
>>
>> This commit help if the forwarding system has to re-segment the data
>> before transition.  Especially if the receiving interface had GRO
>> enabled with checksum offloading and the transmitting interface does
>> not support checksum offloading.
>
> No, the Debian 3.10 kernel doesn't seem to have that commit:
>
> Around line 2859 in skbuff.c I see:
>
>                  if (fskb != skb_shinfo(skb)->frag_list)
>                          continue;
>

Give it a try.  It has helped me in similar situations.  If it helps,
we can get it into the stable tree.

-vlad

^ permalink raw reply

* Re: [PATCH 1/4] wl1251: split wl251 platform data to a separate structure
From: John W. Linville @ 2013-11-15 14:32 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Luciano Coelho, Rob Herring, Pawel Moll, Mark Rutland,
	Stephen Warren, Ian Campbell, Rob Landley, Russell King,
	Felipe Balbi, Sachin Kamat, Greg Kroah-Hartman, Bill Pemberton,
	devicetree, linux-doc, linux-kernel, linux-omap, linux-arm-kernel,
	linux-wireless, netdev, Luciano Coelho
In-Reply-To: <20131114232217.GV10317@atomide.com>

On Thu, Nov 14, 2013 at 03:22:18PM -0800, Tony Lindgren wrote:
> * Sebastian Reichel <sre@debian.org> [131114 15:04]:
> > On Thu, Nov 14, 2013 at 10:51:33AM -0800, Tony Lindgren wrote:
> > > [...]
> > >
> > > If this is not going into v3.13, these will cause conflicts
> > > with the mach-omap2/board-*.c files for v3.14.
> > > 
> > > So it might be best to do a minimal header patch first that
> > > can be merged in by both linux-omap and wireless trees.
> > 
> > I guess this patch is pretty minimal. It also seems to be acked by
> > the involved Maintainers, so maybe just merge Patch 1 without the
> > other patches?
> > 
> > This does not solve the problem with the struct modification from
> > the second patch, but I guess it's the more intrusive patch.
> 
> Once at least the first two patches are ready, how about I queue
> them after -rc1 and set up an immutable branch that can be merged
> in by linux-omap tree and the wireless tree?

That sounds reasonable to me.

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply

* Re: When a TCP segment is split up (to be sent through a TUN device with a small MTU) who should recalculate the checksum?
From: John Hughes @ 2013-11-15 14:31 UTC (permalink / raw)
  To: Vlad Yasevich; +Cc: John Hughes, netdev
In-Reply-To: <52862DBF.2060801@gmail.com>

On 15/11/13 15:20, Vlad Yasevich wrote:
>
> Can you check to see if you have the following patch in your kernel
> commit: 1cdbcb7957cf9e5f841dbcde9b38fd18a804208b
> Author: Simon Horman <horms@verge.net.au>
> Date:   Sun May 19 15:46:49 2013 +0000
>
>     net: Loosen constraints for recalculating checksum in skb_segment()
>
>
> This commit help if the forwarding system has to re-segment the data
> before transition.  Especially if the receiving interface had GRO
> enabled with checksum offloading and the transmitting interface does
> not support checksum offloading.

No, the Debian 3.10 kernel doesn't seem to have that commit:

Around line 2859 in skbuff.c I see:

                 if (fskb != skb_shinfo(skb)->frag_list)
                         continue;

^ permalink raw reply

* Re: When a TCP segment is split up (to be sent through a TUN device with a small MTU) who should recalculate the checksum?
From: Vlad Yasevich @ 2013-11-15 14:20 UTC (permalink / raw)
  To: John Hughes, netdev
In-Reply-To: <5285E0BC.4090402@atlantech.com>

On 11/15/2013 03:52 AM, John Hughes wrote:
> I have two offices, joined by a OpenVPN tunnel.  I've upgraded the
> kernels in the machines running the tunnel to 3.10.  All of a sudden I'm
> getting horrible transmission delays between the two offices.
>
> office1 LAN--------office1 tunnel machine
>                              |
>                              | openvpn tunnel
>                              |
>                      office2 tunnel machine------office2 LAN
>
> What seems to be happening is that packets are arriving at the LAN
> interface of the machine running the tunnel and being combined by
> generic-receive-offload.  These packets then have to be split up again
> as they are too big for the tunnels MTU.
>
> But when the packets are split the TCP checksum doesn't seem to be being
> recalculated, so the systems on the other end of the tunnel ignore them,
> forcing many retries and the observed delays.
>
> For example, here is a large packet coming in on the NIC of the machine
> running the tunnel, followed by a smaller packet ("caronia" is on the
> office 1 LAN, "olympic" is on the office 2 LAN):
>
> 11:59:23.020426 IP caronia.CalvaEDI.COM.33232 >
> olympic.calvaedi.com.ssh: Flags [.], seq 3073:9843, ack 2233, win 148,
> options [nop,nop,TS val 215919291 ecr 1199882508], length 6770
> 11:59:23.041072 IP caronia.CalvaEDI.COM.33232 >
> olympic.calvaedi.com.ssh: Flags [.], seq 9843:11197, ack 2233, win 148,
> options [nop,nop,TS val 215919297 ecr 1199882508], length 1354
>
>
> Then the packet gets sent out on the tunnel as 5 smaller packets:
>
> 11:59:23.020449 IP caronia.CalvaEDI.COM.33232 >
> olympic.calvaedi.com.ssh: Flags [.], seq 3073:4427, ack 2233, win 148,
> options [nop,nop,TS val 215919291 ecr 1199882508], length 1354
> 11:59:23.020534 IP caronia.CalvaEDI.COM.33232 >
> olympic.calvaedi.com.ssh: Flags [.], seq 4427:5781, ack 2233, win 148,
> options [nop,nop,TS val 215919291 ecr 1199882508], length 1354
> 11:59:23.020536 IP caronia.CalvaEDI.COM.33232 >
> olympic.calvaedi.com.ssh: Flags [.], seq 5781:7135, ack 2233, win 148,
> options [nop,nop,TS val 215919291 ecr 1199882508], length 1354
> 11:59:23.020539 IP caronia.CalvaEDI.COM.33232 >
> olympic.calvaedi.com.ssh: Flags [.], seq 7135:8489, ack 2233, win 148,
> options [nop,nop,TS val 215919291 ecr 1199882508], length 1354
> 11:59:23.020543 IP caronia.CalvaEDI.COM.33232 >
> olympic.calvaedi.com.ssh: Flags [.], seq 8489:9843, ack 2233, win 148,
> options [nop,nop,TS val 215919291 ecr 1199882508], length 1354
> 11:59:23.041086 IP caronia.CalvaEDI.COM.33232 >
> olympic.calvaedi.com.ssh: Flags [.], seq 9843:11197, ack 2233, win 148,
> options [nop,nop,TS val 215919297 ecr 1199882508], length 1354
>
>
>
> And this is what the receiving system sees:
>
> 11:59:23.025658 IP (tos 0x8, ttl 62, id 42831, offset 0, flags [DF],
> proto TCP (6), length 1406)
>      caronia.CalvaEDI.COM.33232 > olympic.calvaedi.com.ssh: Flags [.],
> cksum 0x1003 (incorrect -> 0xb1b9), seq 3073:4427, ack 2233, win 148,
> options [nop,nop,TS val 215919291 ecr 1199882508], length 1354
> 11:59:23.025907 IP (tos 0x8, ttl 62, id 42832, offset 0, flags [DF],
> proto TCP (6), length 1406)
>      caronia.CalvaEDI.COM.33232 > olympic.calvaedi.com.ssh: Flags [.],
> cksum 0x1003 (incorrect -> 0x871c), seq 4427:5781, ack 2233, win 148,
> options [nop,nop,TS val 215919291 ecr 1199882508], length 1354
> 11:59:23.025990 IP (tos 0x8, ttl 62, id 42833, offset 0, flags [DF],
> proto TCP (6), length 1406)
>      caronia.CalvaEDI.COM.33232 > olympic.calvaedi.com.ssh: Flags [.],
> cksum 0x1003 (incorrect -> 0x97dd), seq 5781:7135, ack 2233, win 148,
> options [nop,nop,TS val 215919291 ecr 1199882508], length 1354
> 11:59:23.026183 IP (tos 0x8, ttl 62, id 42834, offset 0, flags [DF],
> proto TCP (6), length 1406)
>      caronia.CalvaEDI.COM.33232 > olympic.calvaedi.com.ssh: Flags [.],
> cksum 0x1003 (incorrect -> 0x9961), seq 7135:8489, ack 2233, win 148,
> options [nop,nop,TS val 215919291 ecr 1199882508], length 1354
> 11:59:23.026231 IP (tos 0x8, ttl 62, id 42835, offset 0, flags [DF],
> proto TCP (6), length 1406)
>      caronia.CalvaEDI.COM.33232 > olympic.calvaedi.com.ssh: Flags [.],
> cksum 0x1003 (incorrect -> 0x6a2a), seq 8489:9843, ack 2233, win 148,
> options [nop,nop,TS val 215919291 ecr 1199882508], length 1354
> 11:59:23.046163 IP (tos 0x8, ttl 62, id 42836, offset 0, flags [DF],
> proto TCP (6), length 1406)
>      caronia.CalvaEDI.COM.33232 > olympic.calvaedi.com.ssh: Flags [.],
> cksum 0xd237 (correct), seq 9843:11197, ack 2233, win 148, options
> [nop,nop,TS val 215919297 ecr 1199882508], length 1354
>
>
> The receiving system is, of course, unhappy about that and complains
> that it hasn't got 3073:9843
>
> 11:59:23.045040 IP olympic.calvaedi.com.ssh >
> caronia.CalvaEDI.COM.33232: Flags [.], ack 3073, win 1933, options
> [nop,nop,TS val 1199882514 ecr 215919290,nop,nop,sack 1 {9843:11197}],
> length 0
>
>
> So, when the 6770 byte segment is split up into five 1354 byte segments
> who is supposed to recalculate the checksums?
>
> (This is Debian bug 729567).
>
>

Can you check to see if you have the following patch in your kernel
commit: 1cdbcb7957cf9e5f841dbcde9b38fd18a804208b
Author: Simon Horman <horms@verge.net.au>
Date:   Sun May 19 15:46:49 2013 +0000

     net: Loosen constraints for recalculating checksum in skb_segment()


This commit help if the forwarding system has to re-segment the data
before transition.  Especially if the receiving interface had GRO
enabled with checksum offloading and the transmitting interface does
not support checksum offloading.

-vlad

^ permalink raw reply

* Re: When a TCP segment is split up (to be sent through a TUN device with a small MTU) who should recalculate the checksum?
From: John Hughes @ 2013-11-15 14:02 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: John Hughes, netdev
In-Reply-To: <1384520815.28716.63.camel@edumazet-glaptop2.roam.corp.google.com>

On 15/11/13 14:06, Eric Dumazet wrote:
> On Fri, 2013-11-15 at 09:52 +0100, John Hughes wrote:
>> I have two offices, joined by a OpenVPN tunnel.  I've upgraded the
>> kernels in the machines running the tunnel to 3.10.  All of a sudden I'm
>> getting horrible transmission delays between the two offices.
>>
>> office1 LAN--------office1 tunnel machine
>>                               |
>>                               | openvpn tunnel
>>                               |
>>                       office2 tunnel machine------office2 LAN
>>                                     
>>
>> What seems to be happening is that packets are arriving at the LAN
>> interface of the machine running the tunnel and being combined by
>> generic-receive-offload.  These packets then have to be split up again
>> as they are too big for the tunnels MTU.
>>
>> But when the packets are split the TCP checksum doesn't seem to be being
>> recalculated, so the systems on the other end of the tunnel ignore them,
>> forcing many retries and the observed delays.
>>
>>
> Thanks for the report
>
> It depends on the offload capabilities of the NIC forwarding packets

The NIC is a kernel tun device.
>
> ethtool -k eth0

# ethtool -k tun1
Features for tun1:
rx-checksumming: off [fixed]
tx-checksumming: off
	tx-checksum-ipv4: off [fixed]
	tx-checksum-ip-generic: off [requested on]
	tx-checksum-ipv6: off [fixed]
	tx-checksum-fcoe-crc: off [fixed]
	tx-checksum-sctp: off [fixed]
...

It seems someone has asked the tunnel to compute checksums 
(tx-checksum-ip-generic), but it can't do that.


>
> TCP checksums can be recomputed by tcp_gso_segment() (it was named
> tcp_tso_segment() in linux 3.10), unless the NIC told it was doing the
> checksum computation itself.
>
> ethtool -K eth0 tx off
>
> Should request stack to perform the cheksums.
>
> What is the NIC doing the transmits ?
>
>
>

^ permalink raw reply

* Re: [PATCH 3/3] net: sched: htb: fix calculation of quantum
From: Eric Dumazet @ 2013-11-15 14:01 UTC (permalink / raw)
  To: Yang Yingliang; +Cc: davem, netdev
In-Reply-To: <1384419714-24364-4-git-send-email-yangyingliang@huawei.com>

On Thu, 2013-11-14 at 17:01 +0800, Yang Yingliang wrote:
> Now, 32bit rates may be not the true rate.
> So use rate_bytes_ps which is from
> max(rate32, rate64) to calcualte quantum.
> 
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> ---
>  net/sched/sch_htb.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)

Acked-by: Eric Dumazet <edumazet@google.com>

^ permalink raw reply

* Re: [PATCH] net: sctp: recover a tranport when an ack comes
From: Vlad Yasevich @ 2013-11-15 14:00 UTC (permalink / raw)
  To: Neil Horman
  Cc: Chang Xiangzhong, davem, linux-sctp, netdev, linux-kernel, dreibh,
	ernstgr
In-Reply-To: <20131115123055.GA29877@hmsreliant.think-freely.org>

On 11/15/2013 07:30 AM, Neil Horman wrote:
> On Thu, Nov 14, 2013 at 09:34:55PM -0500, Vlad Yasevich wrote:
>> On 11/14/2013 03:40 PM, Chang Xiangzhong wrote:
>>> Expected Behavior:
>>> When hearing an ack from a tranport/path, set its state to normal/on if it's
>>> in abnormal(__partial_failure__ or inactive) state.
>>>
>>> state machine of tranport->state
>>> Whenever a T3_RTX timer expires, then transport->error_count++.
>>> When (association->pf_retrans < transport->error_count < tranport->pathmaxrtx)
>>> 	transport->state = SCTP_PF //partial failure
>>>
>>> When a heartbeat-ack comes or conventional ack acknowledged its availability,
>>> 	transport->state = SCTP_ON
>>>
>>> Signed-off-by: Chang Xiangzhong <changxiangzhong@gmail.com>
>>> Fixes: 5aa93bcf66f ("sctp: Implement quick failover draft from tsvwg")
>>
>> I don't think this is right.  The spec states:
>>   8.  ACKs for retransmissions do not transition a PF destination back
>>         to Active state, since a sender cannot disambiguate whether the
>>         ack was for the original transmission or the retransmission(s).
>>
>>
>> Now, the proper way to this would would be modify
>> sctp_assoc_control_transport() to transition the transport state to
>> ACTIVE if it was PF transport that was chosen to send data.
>>
>> -vlad
>>
> I agree, this patch doesn't agree with the spec, the only time we transition
> from PF to ACTIVE should be on receipt of ack of new data.

You mean HB ACK, right?  The 02 spec that see on the ietf site doesn't
mention anything about transition on SACKs.  Also, there is no way to
tell right now if the ack is for new or retransmitted data.  We could
mark chunks that are retransmitted though.

>  I'm not even sure if
> we should allow PF transports to be selected to send new data.  Currently a
> potentially failed transport will get ignored when specified, and the stack will
> use the active path in its place.  Only if all transports are PF will a PF
> transport be chosen.

Not even that :(.  If all transports are PF, we are going to camp on
the primary path instead of choosing a PF transport with the lowest
error count.

-vlad

> Neil
>
>>> ---
>>>   net/sctp/outqueue.c |    1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
>>> index 94df758..2557fa5 100644
>>> --- a/net/sctp/outqueue.c
>>> +++ b/net/sctp/outqueue.c
>>> @@ -1517,6 +1517,7 @@ static void sctp_check_transmitted(struct sctp_outq *q,
>>>   			 * active if it is not so marked.
>>>   			 */
>>>   			if ((transport->state == SCTP_INACTIVE ||
>>> +			     transport->state == SCTP_PF ||
>>>   			     transport->state == SCTP_UNCONFIRMED) &&
>>>   			    sctp_cmp_addr_exact(&transport->ipaddr, saddr)) {
>>>   				sctp_assoc_control_transport(
>>>
>>
>>

^ permalink raw reply

* Re: IPv6: Blackhole route support partial ?
From: Hannes Frederic Sowa @ 2013-11-15 13:48 UTC (permalink / raw)
  To: Kamala R; +Cc: davem, linux-kernel, netdev
In-Reply-To: <CAP8BHkF_axGOChL=GigyKxWY1XYr_tQhWFF21ZxpVX9YpRpQSQ@mail.gmail.com>

Hi!

On Fri, Nov 15, 2013 at 03:06:28PM +0530, Kamala R wrote:
> That's right. We don't show the error for IPv4 routes as it follows a
> different path in kernel while dumping the information as compared
> with IPv6. Therefore, "ip route show" does not show an error while "ip
> -6 route show" does.  So it looks to me that this a kernel problem
> which needs to be fixed for consistent behavior. The simplest way to
> fix this seems to be to set the error to zero while dumping the
> information in the v6 path. I have tested this solution and found that
> it works fine. Do you think this is the way to go ?

If I understand you correctly you propose to drop the output of the error
attribute for IPv6 routes too? It is not that important that those two
outputs are identical and if you make a change, please introduce the
error propagation for IPv4 so one can see the socket errors for those
routes, too. I wouldn't drop those for IPv6 just for consistency reasons.

Greetings,

  Hannes

^ permalink raw reply

* Re: [PATCH 0/9] genetlink: reduce ops size and complexity (v2)
From: Johannes Berg @ 2013-11-15 13:23 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, linux-wimax, bsingharora, netfilter-devel,
	alex.bluesman.smirnov, dbaryshkov
In-Reply-To: <1384521515.14295.2.camel@jlt4.sipsolutions.net>

On Fri, 2013-11-15 at 14:18 +0100, Johannes Berg wrote:

> I've been eyeing the multicast groups as well

Of course, there are also *much* fewer mcast groups, so the saving isn't
nearly as big. But we still have some oddball code to register them all,
basically

	err = register_mc_group();
	if (err)
		goto unregister_family;

(since we have to register the family first, afaict)

sometimes a few of those back to back, and it'd be nicer on the users if
that just went away and was

	family.mcast_groups = my_groups;
	family.n_mcast_groups = ARRAY_SIZE(my_groups);
	family.ops = my_ops;
	family.n_ops = ARRAY_SIZE(my_ops);
	return genl_register_family(&family);

since nobody uses them dynamically anyway, afaict.

johannes


^ permalink raw reply

* [PATCH] genetlink: unify registration functions
From: Johannes Berg @ 2013-11-15 13:19 UTC (permalink / raw)
  To: netdev; +Cc: Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

Now that the ops assignment is just two variables rather than a
long list iteration etc., there's no reason to separately export
__genl_register_family() and __genl_register_family_with_ops().

Unify the two functions into __genl_register_family() and make
genl_register_family_with_ops() call it after assigning the ops.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 include/net/genetlink.h | 28 ++++++++++++++++++++++----
 net/netlink/genetlink.c | 53 +++++++++++++++----------------------------------
 2 files changed, 40 insertions(+), 41 deletions(-)

diff --git a/include/net/genetlink.h b/include/net/genetlink.h
index 0b6a144..e96385d 100644
--- a/include/net/genetlink.h
+++ b/include/net/genetlink.h
@@ -131,14 +131,34 @@ static inline int genl_register_family(struct genl_family *family)
 	return __genl_register_family(family);
 }
 
-int __genl_register_family_with_ops(struct genl_family *family,
-				    const struct genl_ops *ops, size_t n_ops);
-
+/**
+ * genl_register_family_with_ops - register a generic netlink family with ops
+ * @family: generic netlink family
+ * @ops: operations to be registered
+ * @n_ops: number of elements to register
+ *
+ * Registers the specified family and operations from the specified table.
+ * Only one family may be registered with the same family name or identifier.
+ *
+ * The family id may equal GENL_ID_GENERATE causing an unique id to
+ * be automatically generated and assigned.
+ *
+ * Either a doit or dumpit callback must be specified for every registered
+ * operation or the function will fail. Only one operation structure per
+ * command identifier may be registered.
+ *
+ * See include/net/genetlink.h for more documenation on the operations
+ * structure.
+ *
+ * Return 0 on success or a negative error code.
+ */
 static inline int genl_register_family_with_ops(struct genl_family *family,
 	const struct genl_ops *ops, size_t n_ops)
 {
 	family->module = THIS_MODULE;
-	return __genl_register_family_with_ops(family, ops, n_ops);
+	family->ops = ops;
+	family->n_ops = n_ops;
+	return __genl_register_family(family);
 }
 
 int genl_unregister_family(struct genl_family *family);
diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index a7c62d3..f07eb56 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -283,12 +283,18 @@ static void genl_unregister_mc_groups(struct genl_family *family)
 		__genl_unregister_mc_group(family, grp);
 }
 
-static int genl_validate_add_ops(struct genl_family *family,
-				 const struct genl_ops *ops,
-				 unsigned int n_ops)
+static int genl_validate_ops(struct genl_family *family)
 {
+	const struct genl_ops *ops = family->ops;
+	unsigned int n_ops = family->n_ops;
 	int i, j;
 
+	if (WARN_ON(n_ops && !ops))
+		return -EINVAL;
+
+	if (!n_ops)
+		return 0;
+
 	for (i = 0; i < n_ops; i++) {
 		if (ops[i].dumpit == NULL && ops[i].doit == NULL)
 			return -EINVAL;
@@ -313,6 +319,9 @@ static int genl_validate_add_ops(struct genl_family *family,
  * The family id may equal GENL_ID_GENERATE causing an unique id to
  * be automatically generated and assigned.
  *
+ * The family's ops array must already be assigned, you can use the
+ * genl_register_family_with_ops() helper function.
+ *
  * Return 0 on success or a negative error code.
  */
 int __genl_register_family(struct genl_family *family)
@@ -325,6 +334,10 @@ int __genl_register_family(struct genl_family *family)
 	if (family->id > GENL_MAX_ID)
 		goto errout;
 
+	err = genl_validate_ops(family);
+	if (err)
+		return err;
+
 	INIT_LIST_HEAD(&family->mcast_groups);
 
 	genl_lock_all();
@@ -373,40 +386,6 @@ errout:
 EXPORT_SYMBOL(__genl_register_family);
 
 /**
- * __genl_register_family_with_ops - register a generic netlink family
- * @family: generic netlink family
- * @ops: operations to be registered
- * @n_ops: number of elements to register
- *
- * Registers the specified family and operations from the specified table.
- * Only one family may be registered with the same family name or identifier.
- *
- * The family id may equal GENL_ID_GENERATE causing an unique id to
- * be automatically generated and assigned.
- *
- * Either a doit or dumpit callback must be specified for every registered
- * operation or the function will fail. Only one operation structure per
- * command identifier may be registered.
- *
- * See include/net/genetlink.h for more documenation on the operations
- * structure.
- *
- * Return 0 on success or a negative error code.
- */
-int __genl_register_family_with_ops(struct genl_family *family,
-	const struct genl_ops *ops, size_t n_ops)
-{
-	int err;
-
-	err = genl_validate_add_ops(family, ops, n_ops);
-	if (err)
-		return err;
-
-	return __genl_register_family(family);
-}
-EXPORT_SYMBOL(__genl_register_family_with_ops);
-
-/**
  * genl_unregister_family - unregister generic netlink family
  * @family: generic netlink family
  *
-- 
1.8.4.rc3

^ permalink raw reply related

* Re: [PATCH 0/9] genetlink: reduce ops size and complexity (v2)
From: Johannes Berg @ 2013-11-15 13:18 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, linux-wimax, bsingharora, netfilter-devel,
	alex.bluesman.smirnov, dbaryshkov
In-Reply-To: <20131114.171204.1979806323628158355.davem@davemloft.net>

On Thu, 2013-11-14 at 17:12 -0500, David Miller wrote:
> From: Johannes Berg <johannes@sipsolutions.net>
> Date: Thu, 14 Nov 2013 17:14:38 +0100
> 
> > As before - reduce the complexity and data/code size of genetlink ops
> > by making them an array rather than a linked list. Most users already
> > use an array thanks to genl_register_family_with_ops(), so convert the
> > remaining ones allowing us to get rid of the list head in each op.
> > 
> > Also make them const, this just makes sense at that point and the security
> > people like making function pointers const as well :-)
> 
> I have to say, this is an absolutely fantastic space usage and
> complexity improvement.

:-)

FWIW, we can also clean up the two registration functions now and make
it just a single one (actually we could have done that before as well,
passing NULL for ops, but hey, now it's even easier). I'll send a patch
for that.

I've been eyeing the multicast groups as well, but the code using them
is nicer if it's not an array. With an array, you'd have to do something
like

send_event(&my_mcast_groups[1]);

instead of

send_event(&my_foo_mcast_group);

which is a bit odd.

We could instead register an array of pointers to the groups:

static const struct mcast_group *my_groups[] = {
	&my_foo_mcast_group,
	...
};

and pass this to the family - that'd still be less space (one pointer
for each group rather than two in a linked list) and still allow all
groups and this array to be const, but it's not quite as big a
saving ...

Thoughts?

johannes



^ permalink raw reply

* Re: When a TCP segment is split up (to be sent through a TUN device with a small MTU) who should recalculate the checksum?
From: Eric Dumazet @ 2013-11-15 13:06 UTC (permalink / raw)
  To: John Hughes; +Cc: netdev
In-Reply-To: <5285E0BC.4090402@atlantech.com>

On Fri, 2013-11-15 at 09:52 +0100, John Hughes wrote:
> I have two offices, joined by a OpenVPN tunnel.  I've upgraded the 
> kernels in the machines running the tunnel to 3.10.  All of a sudden I'm 
> getting horrible transmission delays between the two offices.
> 
> office1 LAN--------office1 tunnel machine
>                              |
>                              | openvpn tunnel
>                              |
>                      office2 tunnel machine------office2 LAN
>                                    
> 
> What seems to be happening is that packets are arriving at the LAN 
> interface of the machine running the tunnel and being combined by 
> generic-receive-offload.  These packets then have to be split up again 
> as they are too big for the tunnels MTU.
> 
> But when the packets are split the TCP checksum doesn't seem to be being 
> recalculated, so the systems on the other end of the tunnel ignore them, 
> forcing many retries and the observed delays.
> 
> For example, here is a large packet coming in on the NIC of the machine 
> running the tunnel, followed by a smaller packet ("caronia" is on the 
> office 1 LAN, "olympic" is on the office 2 LAN):
> 
> 11:59:23.020426 IP caronia.CalvaEDI.COM.33232 > olympic.calvaedi.com.ssh: Flags [.], seq 3073:9843, ack 2233, win 148, options [nop,nop,TS val 215919291 ecr 1199882508], length 6770
> 11:59:23.041072 IP caronia.CalvaEDI.COM.33232 > olympic.calvaedi.com.ssh: Flags [.], seq 9843:11197, ack 2233, win 148, options [nop,nop,TS val 215919297 ecr 1199882508], length 1354
> 
> 
> Then the packet gets sent out on the tunnel as 5 smaller packets:
> 
> 11:59:23.020449 IP caronia.CalvaEDI.COM.33232 > olympic.calvaedi.com.ssh: Flags [.], seq 3073:4427, ack 2233, win 148, options [nop,nop,TS val 215919291 ecr 1199882508], length 1354
> 11:59:23.020534 IP caronia.CalvaEDI.COM.33232 > olympic.calvaedi.com.ssh: Flags [.], seq 4427:5781, ack 2233, win 148, options [nop,nop,TS val 215919291 ecr 1199882508], length 1354
> 11:59:23.020536 IP caronia.CalvaEDI.COM.33232 > olympic.calvaedi.com.ssh: Flags [.], seq 5781:7135, ack 2233, win 148, options [nop,nop,TS val 215919291 ecr 1199882508], length 1354
> 11:59:23.020539 IP caronia.CalvaEDI.COM.33232 > olympic.calvaedi.com.ssh: Flags [.], seq 7135:8489, ack 2233, win 148, options [nop,nop,TS val 215919291 ecr 1199882508], length 1354
> 11:59:23.020543 IP caronia.CalvaEDI.COM.33232 > olympic.calvaedi.com.ssh: Flags [.], seq 8489:9843, ack 2233, win 148, options [nop,nop,TS val 215919291 ecr 1199882508], length 1354
> 11:59:23.041086 IP caronia.CalvaEDI.COM.33232 > olympic.calvaedi.com.ssh: Flags [.], seq 9843:11197, ack 2233, win 148, options [nop,nop,TS val 215919297 ecr 1199882508], length 1354
> 
> 
> 
> And this is what the receiving system sees:
> 
> 11:59:23.025658 IP (tos 0x8, ttl 62, id 42831, offset 0, flags [DF], proto TCP (6), length 1406)
>      caronia.CalvaEDI.COM.33232 > olympic.calvaedi.com.ssh: Flags [.], cksum 0x1003 (incorrect -> 0xb1b9), seq 3073:4427, ack 2233, win 148, options [nop,nop,TS val 215919291 ecr 1199882508], length 1354
> 11:59:23.025907 IP (tos 0x8, ttl 62, id 42832, offset 0, flags [DF], proto TCP (6), length 1406)
>      caronia.CalvaEDI.COM.33232 > olympic.calvaedi.com.ssh: Flags [.], cksum 0x1003 (incorrect -> 0x871c), seq 4427:5781, ack 2233, win 148, options [nop,nop,TS val 215919291 ecr 1199882508], length 1354
> 11:59:23.025990 IP (tos 0x8, ttl 62, id 42833, offset 0, flags [DF], proto TCP (6), length 1406)
>      caronia.CalvaEDI.COM.33232 > olympic.calvaedi.com.ssh: Flags [.], cksum 0x1003 (incorrect -> 0x97dd), seq 5781:7135, ack 2233, win 148, options [nop,nop,TS val 215919291 ecr 1199882508], length 1354
> 11:59:23.026183 IP (tos 0x8, ttl 62, id 42834, offset 0, flags [DF], proto TCP (6), length 1406)
>      caronia.CalvaEDI.COM.33232 > olympic.calvaedi.com.ssh: Flags [.], cksum 0x1003 (incorrect -> 0x9961), seq 7135:8489, ack 2233, win 148, options [nop,nop,TS val 215919291 ecr 1199882508], length 1354
> 11:59:23.026231 IP (tos 0x8, ttl 62, id 42835, offset 0, flags [DF], proto TCP (6), length 1406)
>      caronia.CalvaEDI.COM.33232 > olympic.calvaedi.com.ssh: Flags [.], cksum 0x1003 (incorrect -> 0x6a2a), seq 8489:9843, ack 2233, win 148, options [nop,nop,TS val 215919291 ecr 1199882508], length 1354
> 11:59:23.046163 IP (tos 0x8, ttl 62, id 42836, offset 0, flags [DF], proto TCP (6), length 1406)
>      caronia.CalvaEDI.COM.33232 > olympic.calvaedi.com.ssh: Flags [.], cksum 0xd237 (correct), seq 9843:11197, ack 2233, win 148, options [nop,nop,TS val 215919297 ecr 1199882508], length 1354
> 
> 
> The receiving system is, of course, unhappy about that and complains 
> that it hasn't got 3073:9843
> 
> 11:59:23.045040 IP olympic.calvaedi.com.ssh > caronia.CalvaEDI.COM.33232: Flags [.], ack 3073, win 1933, options [nop,nop,TS val 1199882514 ecr 215919290,nop,nop,sack 1 {9843:11197}], length 0
> 
> 
> So, when the 6770 byte segment is split up into five 1354 byte segments 
> who is supposed to recalculate the checksums?
> 
> (This is Debian bug 729567).
> 

Thanks for the report

It depends on the offload capabilities of the NIC forwarding packets

ethtool -k eth0

TCP checksums can be recomputed by tcp_gso_segment() (it was named
tcp_tso_segment() in linux 3.10), unless the NIC told it was doing the
checksum computation itself.

ethtool -K eth0 tx off

Should request stack to perform the cheksums.

What is the NIC doing the transmits ?

^ permalink raw reply

* Re: [PATCH] net: sctp: recover a tranport when an ack comes
From: Neil Horman @ 2013-11-15 12:30 UTC (permalink / raw)
  To: Vlad Yasevich
  Cc: Chang Xiangzhong, davem, linux-sctp, netdev, linux-kernel, dreibh,
	ernstgr
In-Reply-To: <5285884F.8030907@gmail.com>

On Thu, Nov 14, 2013 at 09:34:55PM -0500, Vlad Yasevich wrote:
> On 11/14/2013 03:40 PM, Chang Xiangzhong wrote:
> >Expected Behavior:
> >When hearing an ack from a tranport/path, set its state to normal/on if it's
> >in abnormal(__partial_failure__ or inactive) state.
> >
> >state machine of tranport->state
> >Whenever a T3_RTX timer expires, then transport->error_count++.
> >When (association->pf_retrans < transport->error_count < tranport->pathmaxrtx)
> >	transport->state = SCTP_PF //partial failure
> >
> >When a heartbeat-ack comes or conventional ack acknowledged its availability,
> >	transport->state = SCTP_ON
> >
> >Signed-off-by: Chang Xiangzhong <changxiangzhong@gmail.com>
> >Fixes: 5aa93bcf66f ("sctp: Implement quick failover draft from tsvwg")
> 
> I don't think this is right.  The spec states:
>  8.  ACKs for retransmissions do not transition a PF destination back
>        to Active state, since a sender cannot disambiguate whether the
>        ack was for the original transmission or the retransmission(s).
> 
> 
> Now, the proper way to this would would be modify
> sctp_assoc_control_transport() to transition the transport state to
> ACTIVE if it was PF transport that was chosen to send data.
> 
> -vlad
> 
I agree, this patch doesn't agree with the spec, the only time we transition
from PF to ACTIVE should be on receipt of ack of new data.  I'm not even sure if
we should allow PF transports to be selected to send new data.  Currently a
potentially failed transport will get ignored when specified, and the stack will
use the active path in its place.  Only if all transports are PF will a PF
transport be chosen.
Neil

> >---
> >  net/sctp/outqueue.c |    1 +
> >  1 file changed, 1 insertion(+)
> >
> >diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
> >index 94df758..2557fa5 100644
> >--- a/net/sctp/outqueue.c
> >+++ b/net/sctp/outqueue.c
> >@@ -1517,6 +1517,7 @@ static void sctp_check_transmitted(struct sctp_outq *q,
> >  			 * active if it is not so marked.
> >  			 */
> >  			if ((transport->state == SCTP_INACTIVE ||
> >+			     transport->state == SCTP_PF ||
> >  			     transport->state == SCTP_UNCONFIRMED) &&
> >  			    sctp_cmp_addr_exact(&transport->ipaddr, saddr)) {
> >  				sctp_assoc_control_transport(
> >
> 
> 

^ permalink raw reply

* [PATCH v2] brcmfmac: fix possible memory leak
From: Geyslan G. Bem @ 2013-11-15 12:21 UTC (permalink / raw)
  To: geyslan
  Cc: Brett Rudley, Arend van Spriel, Franky (Zhenhui) Lin,
	Hante Meuleman, John W. Linville, Pieter-Paul Giesberts,
	Piotr Haber, open list:BROADCOM BRCM8021...,
	open list:BROADCOM BRCM8021..., open list:NETWORKING DRIVERS,
	open list

Free temporary 'chanspecs' avoiding leakage.

Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
---
 drivers/net/wireless/brcm80211/brcmfmac/p2p.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/p2p.c b/drivers/net/wireless/brcm80211/brcmfmac/p2p.c
index d7a9745..fcc5e7c 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/p2p.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/p2p.c
@@ -823,6 +823,7 @@ static s32 brcmf_p2p_run_escan(struct brcmf_cfg80211_info *cfg,
 		}
 		err = brcmf_p2p_escan(p2p, num_nodfs, chanspecs, search_state,
 				      action, P2PAPI_BSSCFG_DEVICE);
+		kfree(chanspecs);
 	}
 exit:
 	if (err)
-- 
1.8.4.2

^ permalink raw reply related

* RE: [PATCH] brcmfmac: fix possible memory leak
From: Hante Meuleman @ 2013-11-15 12:13 UTC (permalink / raw)
  To: Geyslan G. Bem
  Cc: Brett Rudley, Arend Van Spriel, Franky Lin, John W. Linville,
	Pieter-Paul Giesberts, Piotr Haber,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	brcm80211-dev-list,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1384516475-26589-1-git-send-email-geyslan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Good find, wrong solution. The chanspecs is a temporarily variable which should be freed when exiting the function. Not only when there is an error. I personally would have preferred just a  free at the end of the " if (request->n_channels) {". So something like this:

		}
		err = brcmf_p2p_escan(p2p, num_nodfs, chanspecs, search_state,
				      action, P2PAPI_BSSCFG_DEVICE);
+		kfree(chanspecs);
	}

In this case the pointer doesn't have to be initialized to NULL.


Regards,
Hante

-----Original Message-----
From: Geyslan G. Bem [mailto:geyslan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org] 
Sent: vrijdag 15 november 2013 12:54
To: geyslan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Cc: Brett Rudley; Arend Van Spriel; Franky Lin; Hante Meuleman; John W. Linville; Pieter-Paul Giesberts; Piotr Haber; linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; brcm80211-dev-list; netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: [PATCH] brcmfmac: fix possible memory leak

In case of error free 'chanspecs'.

Signed-off-by: Geyslan G. Bem <geyslan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/net/wireless/brcm80211/brcmfmac/p2p.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/p2p.c b/drivers/net/wireless/brcm80211/brcmfmac/p2p.c
index d7a9745..aea2c2e 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/p2p.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/p2p.c
@@ -771,7 +771,7 @@ static s32 brcmf_p2p_run_escan(struct brcmf_cfg80211_info *cfg,
 	struct brcmf_cfg80211_vif *vif;
 	struct net_device *dev = NULL;
 	int i, num_nodfs = 0;
-	u16 *chanspecs;
+	u16 *chanspecs = NULL;
 
 	brcmf_dbg(TRACE, "enter\n");
 
@@ -825,8 +825,10 @@ static s32 brcmf_p2p_run_escan(struct brcmf_cfg80211_info *cfg,
 				      action, P2PAPI_BSSCFG_DEVICE);
 	}
 exit:
-	if (err)
+	if (err) {
 		brcmf_err("error (%d)\n", err);
+		kfree(chanspecs);
+	}
 	return err;
 }
 
-- 
1.8.4.2



--
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 related

* [PATCH] brcmfmac: fix possible memory leak
From: Geyslan G. Bem @ 2013-11-15 11:54 UTC (permalink / raw)
  To: geyslan
  Cc: Brett Rudley, Arend van Spriel, Franky (Zhenhui) Lin,
	Hante Meuleman, John W. Linville, Pieter-Paul Giesberts,
	Piotr Haber, open list:BROADCOM BRCM8021...,
	open list:BROADCOM BRCM8021..., open list:NETWORKING DRIVERS,
	open list

In case of error free 'chanspecs'.

Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
---
 drivers/net/wireless/brcm80211/brcmfmac/p2p.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/p2p.c b/drivers/net/wireless/brcm80211/brcmfmac/p2p.c
index d7a9745..aea2c2e 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/p2p.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/p2p.c
@@ -771,7 +771,7 @@ static s32 brcmf_p2p_run_escan(struct brcmf_cfg80211_info *cfg,
 	struct brcmf_cfg80211_vif *vif;
 	struct net_device *dev = NULL;
 	int i, num_nodfs = 0;
-	u16 *chanspecs;
+	u16 *chanspecs = NULL;
 
 	brcmf_dbg(TRACE, "enter\n");
 
@@ -825,8 +825,10 @@ static s32 brcmf_p2p_run_escan(struct brcmf_cfg80211_info *cfg,
 				      action, P2PAPI_BSSCFG_DEVICE);
 	}
 exit:
-	if (err)
+	if (err) {
 		brcmf_err("error (%d)\n", err);
+		kfree(chanspecs);
+	}
 	return err;
 }
 
-- 
1.8.4.2

^ permalink raw reply related

* Re: [ovs-dev] [PATCH] linux: Signal datapath that unaligned Netlink message can be received
From: Jesse Gross @ 2013-11-15 10:29 UTC (permalink / raw)
  To: Thomas Graf
  Cc: Ben Pfaff, dev@openvswitch.org, Ben Hutchings, fleitner,
	dborkmann, netdev
In-Reply-To: <5285EDA1.5060609@redhat.com>

On Fri, Nov 15, 2013 at 5:47 PM, Thomas Graf <tgraf@redhat.com> wrote:
> On 11/15/2013 10:32 AM, Jesse Gross wrote:
>>
>> On Wed, Nov 13, 2013 at 5:46 PM, Thomas Graf <tgraf@redhat.com> wrote:
>>>
>>> On 11/13/2013 07:11 AM, Jesse Gross wrote:
>>>>
>>>>
>>>> On Mon, Nov 11, 2013 at 11:53 PM, Thomas Graf <tgraf@redhat.com> wrote:
>>>>>
>>>>>
>>>>> On 11/11/2013 04:50 PM, Ben Pfaff wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Mon, Nov 11, 2013 at 04:36:24PM +0100, Thomas Graf wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Following commit (''netlink: Do not enforce alignment of last Netlink
>>>>>>> attribute''), signal the ability to receive unaligned Netlink
>>>>>>> messages
>>>>>>> to the datapath to enable utilization of zerocopy optimizations.
>>>>>>>
>>>>>>> Signed-off-by: Thomas Graf <tgraf@redhat.com>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Seems OK from a userspace point of view.  I am a little concerned that
>>>>>> downgrading userspace without deleting and re-creating the datapath
>>>>>> (e.g. via "force-reload-kmod") will result in a totally broken setup
>>>>>> since userspace will then drop every packet from the kernel.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Is that something that occurs occasionally in installations? Utilizing
>>>>> the version field in the genl header could be used to track this and
>>>>> clear user_features.
>>>>
>>>>
>>>>
>>>> It's probably a good idea. I could see us having more of these
>>>> features flags in the future (although obviously we should try to
>>>> avoid them if possible) and, as Ben said, it would potentially lead to
>>>> a bad state otherwise.
>>>>
>>>> I'm not sure exactly what you have in mind though, can you elaborate a
>>>> little?
>>>
>>>
>>>
>>> My initial thought was to use a version field to notice the replacement
>>> of user space. On second thought that is not required, modifying user
>>> space to provide the user features in OVS_DP_CMD_GET as well will
>>> overwrite the features. Resetting user_features to 0 if not features are
>>> provided will provide backwards compatibility to versions not aware of
>>> user features yet. Thoughts?
>>
>>
>> Hmm, it doesn't really seem ideal to make DP_CMD_GET change settings
>> as it's probably not the expected behavior for most users of the
>> command. One example of where this could be a problem is if it is
>> called from both ovs-dpctl and ovs-vswitch. Usually these would be
>> have the same capabilities but I'm not sure that it's strictly
>> required in all cases. DP_CMD_SET seems like the ideal place to put it
>> but I guess we don't use that at all on existing versions of OVS
>> userspace.
>
>
> Agreed. Ideal and clean is a DP_CMD_SET lookup-or-create that updates
> the user features. That will leave one OVS version to be non backwards
> compatible in terms of downgrading user features.
>
> For that reason we can bump OVS_DATAPATH_VERSION to version 2 and reset
> the user features if we receive a DP_CMD_GET version 1.
>
> It's not a problem if we reset the user features unnecessarily in some
> corner cases such as mismatching ovs-dpctl and vswitchd. The next
> iteration of the patchset has support for memory mapped netlink i/o
> which would still apply.
>
> If you agree to this then I will cook this up.

That seems like a reasonable solution to me.

^ permalink raw reply

* [PATCH net] bonding: set primary_reselect in LB and AB mode
From: Ding Tianhong @ 2013-11-15 10:02 UTC (permalink / raw)
  To: Jay Vosburgh, Andy Gospodarek, David S. Miller,
	Nikolay Aleksandrov, Veaceslav Falico, Netdev

The primary_reselect only reselection for the primary slave,
but the primary slave only support for ALB, TLB and AB mode,
so we sould set the primary_reselect for these mode.

to fix this: Add a check for ALB, TLB and AB mode in
bonding_store_primary_reselect, avoid to select active slave
again in other modes.

Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
 drivers/net/bonding/bond_sysfs.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 47749c9..cac2291 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -1141,6 +1141,13 @@ static ssize_t bonding_store_primary_reselect(struct device *d,
 	if (!rtnl_trylock())
 		return restart_syscall();
 
+	if (!USES_PRIMARY(bond->params.mode)) {
+		pr_info("%s: Unable to set primary reselect; %s is in mode %d\n",
+			bond->dev->name, bond->dev->name, bond->params.mode);
+		ret = -EINVAL;
+		goto out;
+	}
+
 	new_value = bond_parse_parm(buf, pri_reselect_tbl);
 	if (new_value < 0)  {
 		pr_err("%s: Ignoring invalid primary_reselect value %.*s.\n",
-- 
1.7.12

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox