Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 1/3] xen-netback: remove page tracking facility
From: Wei Liu @ 2013-08-26 11:59 UTC (permalink / raw)
  To: netdev; +Cc: xen-devel, ian.campbell, msw, konrad.wilk, annie.li, Wei Liu
In-Reply-To: <1377518379-26503-1-git-send-email-wei.liu2@citrix.com>

The data flow from DomU to DomU on the same host in current copying
scheme with tracking facility:

       copy
DomU --------> Dom0          DomU
 |                            ^
 |____________________________|
             copy

The page in Dom0 is a page with valid MFN. So we can always copy from
page Dom0, thus removing the need for a tracking facility.

       copy           copy
DomU --------> Dom0 -------> DomU

Simple iperf test shows no performance regression (obviously we copy
twice either way):

  W/  tracking: ~5.3Gb/s
  W/o tracking: ~5.4Gb/s

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Matt Wilson <msw@amazon.com>
---
 drivers/net/xen-netback/netback.c |   77 +------------------------------------
 1 file changed, 2 insertions(+), 75 deletions(-)

diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index 64828de..91f163d 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -95,21 +95,6 @@ struct netbk_rx_meta {
 
 #define MAX_BUFFER_OFFSET PAGE_SIZE
 
-/* extra field used in struct page */
-union page_ext {
-	struct {
-#if BITS_PER_LONG < 64
-#define IDX_WIDTH   8
-#define GROUP_WIDTH (BITS_PER_LONG - IDX_WIDTH)
-		unsigned int group:GROUP_WIDTH;
-		unsigned int idx:IDX_WIDTH;
-#else
-		unsigned int group, idx;
-#endif
-	} e;
-	void *mapping;
-};
-
 struct xen_netbk {
 	wait_queue_head_t wq;
 	struct task_struct *task;
@@ -214,45 +199,6 @@ static inline unsigned long idx_to_kaddr(struct xen_netbk *netbk,
 	return (unsigned long)pfn_to_kaddr(idx_to_pfn(netbk, idx));
 }
 
-/* extra field used in struct page */
-static inline void set_page_ext(struct page *pg, struct xen_netbk *netbk,
-				unsigned int idx)
-{
-	unsigned int group = netbk - xen_netbk;
-	union page_ext ext = { .e = { .group = group + 1, .idx = idx } };
-
-	BUILD_BUG_ON(sizeof(ext) > sizeof(ext.mapping));
-	pg->mapping = ext.mapping;
-}
-
-static int get_page_ext(struct page *pg,
-			unsigned int *pgroup, unsigned int *pidx)
-{
-	union page_ext ext = { .mapping = pg->mapping };
-	struct xen_netbk *netbk;
-	unsigned int group, idx;
-
-	group = ext.e.group - 1;
-
-	if (group < 0 || group >= xen_netbk_group_nr)
-		return 0;
-
-	netbk = &xen_netbk[group];
-
-	idx = ext.e.idx;
-
-	if ((idx < 0) || (idx >= MAX_PENDING_REQS))
-		return 0;
-
-	if (netbk->mmap_pages[idx] != pg)
-		return 0;
-
-	*pgroup = group;
-	*pidx = idx;
-
-	return 1;
-}
-
 /*
  * This is the amount of packet we copy rather than map, so that the
  * guest can't fiddle with the contents of the headers while we do
@@ -453,12 +399,6 @@ static void netbk_gop_frag_copy(struct xenvif *vif, struct sk_buff *skb,
 {
 	struct gnttab_copy *copy_gop;
 	struct netbk_rx_meta *meta;
-	/*
-	 * These variables are used iff get_page_ext returns true,
-	 * in which case they are guaranteed to be initialized.
-	 */
-	unsigned int uninitialized_var(group), uninitialized_var(idx);
-	int foreign = get_page_ext(page, &group, &idx);
 	unsigned long bytes;
 
 	/* Data must not cross a page boundary. */
@@ -494,20 +434,9 @@ static void netbk_gop_frag_copy(struct xenvif *vif, struct sk_buff *skb,
 
 		copy_gop = npo->copy + npo->copy_prod++;
 		copy_gop->flags = GNTCOPY_dest_gref;
-		if (foreign) {
-			struct xen_netbk *netbk = &xen_netbk[group];
-			struct pending_tx_info *src_pend;
+		copy_gop->source.domid = DOMID_SELF;
+		copy_gop->source.u.gmfn = virt_to_mfn(page_address(page));
 
-			src_pend = &netbk->pending_tx_info[idx];
-
-			copy_gop->source.domid = src_pend->vif->domid;
-			copy_gop->source.u.ref = src_pend->req.gref;
-			copy_gop->flags |= GNTCOPY_source_gref;
-		} else {
-			void *vaddr = page_address(page);
-			copy_gop->source.domid = DOMID_SELF;
-			copy_gop->source.u.gmfn = virt_to_mfn(vaddr);
-		}
 		copy_gop->source.offset = offset;
 		copy_gop->dest.domid = vif->domid;
 
@@ -1047,7 +976,6 @@ static struct page *xen_netbk_alloc_page(struct xen_netbk *netbk,
 	page = alloc_page(GFP_KERNEL|__GFP_COLD);
 	if (!page)
 		return NULL;
-	set_page_ext(page, netbk, pending_idx);
 	netbk->mmap_pages[pending_idx] = page;
 	return page;
 }
@@ -1155,7 +1083,6 @@ static struct gnttab_copy *xen_netbk_get_requests(struct xen_netbk *netbk,
 		first->req.offset = 0;
 		first->req.size = dst_offset;
 		first->head = start_idx;
-		set_page_ext(page, netbk, head_idx);
 		netbk->mmap_pages[head_idx] = page;
 		frag_set_pending_idx(&frags[shinfo->nr_frags], head_idx);
 	}
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH net-next 0/3] xen-netback: switch to NAPI + kthread 1:1 model
From: Wei Liu @ 2013-08-26 11:59 UTC (permalink / raw)
  To: netdev; +Cc: xen-devel, ian.campbell, msw, konrad.wilk, annie.li, Wei Liu

This series implements NAPI + kthread 1:1 model for Xen netback.

This model
 - provides better scheduling fairness among vifs
 - is prerequisite for implementing multiqueue for Xen network driver

The second patch has the real meat:
 - make use of NAPI to mitigate interrupt
 - kthreads are not bound to CPUs any more, so that we can take
   advantage of backend scheduler and trust it to do the right thing

Benchmark is done on a Dell T3400 workstation with 4 cores, running 4
DomUs. Netserver runs in Dom0. DomUs do netperf to Dom0 with
following command: /root/netperf -H Dom0 -fm -l120

IRQs are distributed to 4 cores by hand in the new model, while in the
old model vifs are automatically distributed to 4 kthreads.

* New model
%Cpu0  :  0.5 us, 20.3 sy,  0.0 ni, 28.9 id,  0.0 wa,  0.0 hi, 24.4 si, 25.9 st
%Cpu1  :  0.5 us, 17.8 sy,  0.0 ni, 28.8 id,  0.0 wa,  0.0 hi, 27.7 si, 25.1 st
%Cpu2  :  0.5 us, 18.8 sy,  0.0 ni, 30.7 id,  0.0 wa,  0.0 hi, 22.9 si, 27.1 st
%Cpu3  :  0.0 us, 20.1 sy,  0.0 ni, 30.4 id,  0.0 wa,  0.0 hi, 22.7 si, 26.8 st
Throughputs: 2027.89 2025.95 2018.57 2016.23 aggregated: 8088.64

* Old model
%Cpu0  :  0.5 us, 68.8 sy,  0.0 ni, 16.1 id,  0.5 wa,  0.0 hi,  2.8 si, 11.5 st
%Cpu1  :  0.4 us, 45.1 sy,  0.0 ni, 31.1 id,  0.4 wa,  0.0 hi,  2.1 si, 20.9 st
%Cpu2  :  0.9 us, 44.8 sy,  0.0 ni, 30.9 id,  0.0 wa,  0.0 hi,  1.3 si, 22.2 st
%Cpu3  :  0.8 us, 46.4 sy,  0.0 ni, 28.3 id,  1.3 wa,  0.0 hi,  2.1 si, 21.1 st
Throughputs: 1899.14 2280.43 1963.33 1893.47 aggregated: 8036.37

We can see that the impact is mainly on CPU usage. The new model moves
processing from kthread to NAPI (software interrupt).

Wei Liu (3):
  xen-netback: remove page tracking facility
  xen-netback: switch to NAPI + kthread 1:1 model
  xen-netback: rename functions

 drivers/net/xen-netback/common.h    |  150 +++++--
 drivers/net/xen-netback/interface.c |  135 ++++--
 drivers/net/xen-netback/netback.c   |  833 +++++++++++------------------------
 3 files changed, 445 insertions(+), 673 deletions(-)

-- 
1.7.10.4

^ permalink raw reply

* Re: [PATCH net-next] {ipv4,xfrm}: Introduce xfrm_tunnel_notifier for xfrm tunnel mode callback
From: Steffen Klassert @ 2013-08-26 11:35 UTC (permalink / raw)
  To: Fan Du; +Cc: saurabh.mohan, herbert, davem, netdev
In-Reply-To: <1377240424-11758-1-git-send-email-fan.du@windriver.com>

On Fri, Aug 23, 2013 at 02:47:04PM +0800, Fan Du wrote:
> Some thoughts on IPv4 VTI implementation:
> 
> The connection between VTI receiving part and xfrm tunnel mode input process
> is hardly a "xfrm_tunnel", xfrm_tunnel is used in places where, e.g ipip/sit
> and xfrm4_tunnel, acts like a true "tunnel" device.
> 
> In addition, IMHO, VTI doesn't need vti_err to do something meaningful, as all
> VTI needs is just a notifier to be called whenever xfrm_input ingress a packet
> to update statistics.
> 
> So this patch introduce xfrm_tunnel_notifier and meanwhile wipe out vti_erri
> code.

Btw. who calls vti_err()? I don't see a hook which would call the vti
error handler. I'm still not absolutely sure whether we need it or not,
but we should either remove it or add a hook to call it.

^ permalink raw reply

* Re: [PATCH ipsec-next] xfrm: announce deleation of temporary SA
From: Steffen Klassert @ 2013-08-26 10:54 UTC (permalink / raw)
  To: Nicolas Dichtel; +Cc: herbert, davem, netdev
In-Reply-To: <1377265568-10959-1-git-send-email-nicolas.dichtel@6wind.com>

On Fri, Aug 23, 2013 at 03:46:08PM +0200, Nicolas Dichtel wrote:
> Creation of temporary SA are announced by netlink, but there is no notification
> for the deletion.
> This patch fix this asymmetric situation.
> 
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

Looks good, applied to ipsec-next. Thanks!

^ permalink raw reply

* Re: Problematic commits in the ipsec tree
From: Hannes Frederic Sowa @ 2013-08-26 10:46 UTC (permalink / raw)
  To: Steffen Klassert; +Cc: David Miller, netdev
In-Reply-To: <20130826094145.GK26773@secunet.com>

On Mon, Aug 26, 2013 at 11:41:45AM +0200, Steffen Klassert wrote:
> On Fri, Aug 23, 2013 at 02:49:11PM +0200, Hannes Frederic Sowa wrote:
> > 
> > This could be the replacement for patch 1/2 to reassemble old behaviour
> > without touching ip6_skb_dst_mtu if the socket type is not an IPv6 one.
> > 
> > I would still like to look if we could correctly handle *_PMTUDISC_PROBE one
> > day and fallback to dst_mtu(dst->path) if possible. So I don't know if
> > removing xfrm_skb_dst_mtu is good style and would just make churn in the git
> > history. What do you think?
> 
> Currently I think we can call dst_mtu() unconditionally from
> __xfrm6_output(), then we would not need xfrm_skb_dst_mtu().
> But this needs further investigation, IPsec pmtu discovery
> was frequently broken in the past and I don't want to break
> it again.

My idea was something like

|	  struct ipv6_pinfo *np = ...;
|         int mtu = (np && np->pmtudisc == IPV6_PMTUDISC_PROBE) ?
|                   dst_mtu(skb_dst(skb)->path) : dst_mtu(skb_dst(skb));

But I don't know if this does actually anything good and where the dispatch of
dst_mtu goes to. My idea was to avoid the dst_metric_raw(dst, RTAX_MTU)
call in xfrm_mtu in case of IPV6_PMTUDISC_PROBE.

> > [PATCH ipsec 1/2] xfrm: revert ipv4 mtu determination to dst_mtu
> > 
> > In commit 0ea9d5e3e0e03a63b11392f5613378977dae7eca ("xfrm: introduce
> > helper for safe determination of mtu") I switched the determination of
> > ipv4 mtus from dst_mtu to ip_skb_dst_mtu. This was an error because in
> > case of IP_PMTUDISC_PROBE we fall back to the interface mtu, which is
> > never correct for ipv4 ipsec.
> > 
> > This patch partly reverts 0ea9d5e3e0e03a63b11392f5613378977dae7eca
> > ("xfrm: introduce helper for safe determination of mtu").
> > 
> 
> I think with this and you other patch, we get the all the
> interfamily tunnel problems fixed for now. Everything else
> should be done in ipsec-next.

Fully ACK.

> Please resend the whole patchset, so we can get it fixed soon.
> 
> Tanks a lot!

Sorry for holding back your tree for so long to get merged.

Thanks,

  Hannes

^ permalink raw reply

* Re: [PATCH 2/2] ipv6: set skb->protocol on tcp, raw and ip6_append_data genereated skbs
From: Hannes Frederic Sowa @ 2013-08-26 10:34 UTC (permalink / raw)
  To: netdev, steffen.klassert, eric.dumazet
In-Reply-To: <20130826103123.GA30508@order.stressinduktion.org>

> Subject: [PATCH 2/2] ipv6: set skb->protocol on tcp, raw and ip6_append_data genereated skbs

Should be [PATCH ipsec 2/2], of course.

^ permalink raw reply

* [PATCH 2/2] ipv6: set skb->protocol on tcp, raw and ip6_append_data genereated skbs
From: Hannes Frederic Sowa @ 2013-08-26 10:31 UTC (permalink / raw)
  To: netdev; +Cc: steffen.klassert, eric.dumazet

Currently we don't initialize skb->protocol when transmitting data via
tcp, raw(with and without inclhdr) or udp+ufo or appending data directly
to the socket transmit queue (via ip6_append_data). This needs to be
done so that we can get the correct mtu in the xfrm layer.

Setting of skb->protocol happens only in functions where we also have
a transmitting socket and a new skb, so we don't overwrite old values.

Cc: Steffen Klassert <steffen.klassert@secunet.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Acked-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 net/ipv6/ip6_output.c | 3 +++
 net/ipv6/raw.c        | 1 +
 2 files changed, 4 insertions(+)

diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 6e3ddf8..e7ceb6c 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -238,6 +238,7 @@ int ip6_xmit(struct sock *sk, struct sk_buff *skb, struct flowi6 *fl6,
 	hdr->saddr = fl6->saddr;
 	hdr->daddr = *first_hop;
 
+	skb->protocol = htons(ETH_P_IPV6);
 	skb->priority = sk->sk_priority;
 	skb->mark = sk->sk_mark;
 
@@ -1057,6 +1058,7 @@ static inline int ip6_ufo_append_data(struct sock *sk,
 		/* initialize protocol header pointer */
 		skb->transport_header = skb->network_header + fragheaderlen;
 
+		skb->protocol = htons(ETH_P_IPV6);
 		skb->ip_summed = CHECKSUM_PARTIAL;
 		skb->csum = 0;
 	}
@@ -1359,6 +1361,7 @@ alloc_new_skb:
 			/*
 			 *	Fill in the control structures
 			 */
+			skb->protocol = htons(ETH_P_IPV6);
 			skb->ip_summed = CHECKSUM_NONE;
 			skb->csum = 0;
 			/* reserve for fragmentation and ipsec header */
diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c
index c45f7a5..cdaed47 100644
--- a/net/ipv6/raw.c
+++ b/net/ipv6/raw.c
@@ -628,6 +628,7 @@ static int rawv6_send_hdrinc(struct sock *sk, void *from, int length,
 		goto error;
 	skb_reserve(skb, hlen);
 
+	skb->protocol = htons(ETH_P_IPV6);
 	skb->priority = sk->sk_priority;
 	skb->mark = sk->sk_mark;
 	skb_dst_set(skb, &rt->dst);
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH ipsec 1/2] xfrm: revert ipv4 mtu determination to dst_mtu
From: Hannes Frederic Sowa @ 2013-08-26 10:31 UTC (permalink / raw)
  To: netdev; +Cc: steffen.klassert

In commit 0ea9d5e3e0e03a63b11392f5613378977dae7eca ("xfrm: introduce
helper for safe determination of mtu") I switched the determination of
ipv4 mtus from dst_mtu to ip_skb_dst_mtu. This was an error because in
case of IP_PMTUDISC_PROBE we fall back to the interface mtu, which is
never correct for ipv4 ipsec.

This patch partly reverts 0ea9d5e3e0e03a63b11392f5613378977dae7eca
("xfrm: introduce helper for safe determination of mtu").

Cc: Steffen Klassert <steffen.klassert@secunet.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 include/net/xfrm.h      | 12 ------------
 net/ipv4/xfrm4_output.c |  2 +-
 net/ipv6/xfrm6_output.c |  8 +++++---
 3 files changed, 6 insertions(+), 16 deletions(-)

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index ac5b025..e823786 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -20,7 +20,6 @@
 #include <net/route.h>
 #include <net/ipv6.h>
 #include <net/ip6_fib.h>
-#include <net/ip6_route.h>
 #include <net/flow.h>
 
 #include <linux/interrupt.h>
@@ -1724,15 +1723,4 @@ static inline int xfrm_mark_put(struct sk_buff *skb, const struct xfrm_mark *m)
 	return ret;
 }
 
-static inline int xfrm_skb_dst_mtu(struct sk_buff *skb)
-{
-	struct sock *sk = skb->sk;
-
-	if (sk && skb->protocol == htons(ETH_P_IPV6))
-		return ip6_skb_dst_mtu(skb);
-	else if (sk && skb->protocol == htons(ETH_P_IP))
-		return ip_skb_dst_mtu(skb);
-	return dst_mtu(skb_dst(skb));
-}
-
 #endif	/* _NET_XFRM_H */
diff --git a/net/ipv4/xfrm4_output.c b/net/ipv4/xfrm4_output.c
index 80baf4a..baa0f63 100644
--- a/net/ipv4/xfrm4_output.c
+++ b/net/ipv4/xfrm4_output.c
@@ -28,7 +28,7 @@ static int xfrm4_tunnel_check_size(struct sk_buff *skb)
 	if (!(ip_hdr(skb)->frag_off & htons(IP_DF)) || skb->local_df)
 		goto out;
 
-	mtu = xfrm_skb_dst_mtu(skb);
+	mtu = dst_mtu(skb_dst(skb));
 	if (skb->len > mtu) {
 		if (skb->sk)
 			xfrm_local_error(skb, mtu);
diff --git a/net/ipv6/xfrm6_output.c b/net/ipv6/xfrm6_output.c
index e092e30..6cd625e 100644
--- a/net/ipv6/xfrm6_output.c
+++ b/net/ipv6/xfrm6_output.c
@@ -140,10 +140,12 @@ static int __xfrm6_output(struct sk_buff *skb)
 {
 	struct dst_entry *dst = skb_dst(skb);
 	struct xfrm_state *x = dst->xfrm;
-	int mtu = xfrm_skb_dst_mtu(skb);
+	int mtu;
 
-	if (mtu < IPV6_MIN_MTU)
-		mtu = IPV6_MIN_MTU;
+	if (skb->protocol == htons(ETH_P_IPV6))
+		mtu = ip6_skb_dst_mtu(skb);
+	else
+		mtu = dst_mtu(skb_dst(skb));
 
 	if (skb->len > mtu && xfrm6_local_dontfrag(skb)) {
 		xfrm6_local_rxpmtu(skb, mtu);
-- 
1.8.3.1

^ permalink raw reply related

* Re: Can linux kernel bridge forward 802.1q tagged vlan packets?
From: Stephan von Krawczynski @ 2013-08-26 10:21 UTC (permalink / raw)
  To: Jeff Kirsher
  Cc: bridge, Marek Kierdelewicz, mitch.a.williams, Stephen Hemminger,
	netdev, Carolyn Wyborny
In-Reply-To: <CAL3LdT62oN3hdrDy2DL_Mk9+vUAsRzxaSjY5O87HbHgsjG+V6Q@mail.gmail.com>

On Thu, 22 Aug 2013 21:58:58 -0700
Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:

> Adding Carolyn Wyborny (igb maintainer) and Mitch Williams (igbvf maintainer)...
> 
> On Thu, Aug 22, 2013 at 9:22 PM, Stephan von Krawczynski
> <skraw@ithnet.com> wrote:
> > On Thu, 22 Aug 2013 15:19:41 -0700
> > Stephen Hemminger <stephen@networkplumber.org> wrote:
> >
> >> On Thu, 22 Aug 2013 23:49:41 +0200
> >> Marek Kierdelewicz <marek@piasta.pl> wrote:
> >>
> >> > Hi,
> >> >
> >> > >My general idea is to bridge tagged vlan packets from a physical
> >> > >interface (intel) to a virtual interface (virtio in qemu) and not
> >> > >losing the tags, so the qemu guest can use vconfig and friends to get
> >> > >some vlan interfaces. Is this possible and are there any additional
> >> > >steps necessary besides the usual bridge configuration?
> >> >
> >> > I'm using this setup with KVM virtualization (Qemu's successor). It
> >> > didn't work with default nic type for guest (realtek I think) as 802.1q
> >> > tags were not preserved properly in one of the directions. I don't
> >> > remember specifics. Anyway using e1000 driver instead of default fixed
> >> > it. Works like a charm.
> >>
> >> KVM e1000 Vnic doesn't do VLAN offloading at least in my experience.
> >> Virtio works fine.
> >
> > Ok guys, here is one of the major questions in this issue:
> >
> > How did you manage to convince the _host_ interface driver (in my setup "igb"
> > from intel) to get the vlan tagged packets from the LAN port. igb has a vlan
> > filter function built in and reads _no_ tagged packets at all if the driver
> > does not tell it to. And in case of "up-ing" the interface for attachment to a
> > bridge it does not configure this vlan filter.
> > How is this expected to work?
> >
> > --
> > Regards,
> > Stephan

Thanks Jeff,

so, since we are all here now :-) who can explain how one can tell either the
bridge or the attached network device to include certain VLAN IDs in its
HW filters so that these can be forwarded through the bridge?
>From the current infos it looks like a cheap network card with no HW filters
would just work (if I understood Stephen). But since most servers have either
intel cards or intel onboard chips (and they are brilliant in performance btw)
me and probably lots of others seem to need some solution that isn't currently
available. I would love to see something that really allows the card to still
use the filters and not switch it in complete promiscous mode so that every
packets floats in.
Any ideas?

-- 
Regards,
Stephan

^ permalink raw reply

* Re: sun4i_handle_irq: WARNING at net/ipv4/tcp_input.c:2711 tcp_fastretrans_alert
From: Richard Genoud @ 2013-08-26 10:19 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Stefan Roese, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <20130823182116.GE1230@lukather>

2013/8/23 Maxime Ripard <maxime.ripard@free-electrons.com>:
> Hi Richard,
>
> On Wed, Aug 21, 2013 at 11:47:20AM +0200, Richard GENOUD wrote:
>> Hi Maxime, Stephan
>>
>> I just realise that, *sometimes*, I have some warnings on my cubieboard
>> (6 since the 22 of july, and the board is runnning 24/24).
>
> Wow, I'm impressed it worked this fine actually :)
I use to host my wiky, ssh and xmpp servers (nfsroot on my NAS) and
yep, it is rock solid so far !

>
>> It has happened also on 3.10 + emac patches.
>> Here it is:
>> [27224.060000] ------------[ cut here ]------------
>> [27224.070000] WARNING: CPU: 0 PID: 0 at net/ipv4/tcp_input.c:2711 tcp_fastretrans_alert+0x717/0x734()
>> [27224.080000] CPU: 0 PID: 0 Comm: swapper Not tainted 3.11.0-rc6-00010-g54f11ec #8
>> [27224.080000] [<c000f231>] (unwind_backtrace+0x1/0x98) from [<c000e2b7>] (show_stack+0xb/0xc)
>> [27224.090000] [<c000e2b7>] (show_stack+0xb/0xc) from [<c0013cd3>] (warn_slowpath_common+0x43/0x5c)
>> [27224.100000] [<c0013cd3>] (warn_slowpath_common+0x43/0x5c) from [<c0013d6d>] (warn_slowpath_null+0x11/0x14)
>> [27224.110000] [<c0013d6d>] (warn_slowpath_null+0x11/0x14) from [<c015eadf>] (tcp_fastretrans_alert+0x717/0x734)
>> [27224.120000] [<c015eadf>] (tcp_fastretrans_alert+0x717/0x734) from [<c015f32b>] (tcp_ack+0x6af/0x984)
>> [27224.130000] [<c015f32b>] (tcp_ack+0x6af/0x984) from [<c015fcab>] (tcp_rcv_established+0x93/0x3ec)
>> [27224.140000] [<c015fcab>] (tcp_rcv_established+0x93/0x3ec) from [<c0165621>] (tcp_v4_do_rcv+0xa1/0x160)
>> [27224.150000] [<c0165621>] (tcp_v4_do_rcv+0xa1/0x160) from [<c0167129>] (tcp_v4_rcv+0x4e9/0x500)
>> [27224.160000] [<c0167129>] (tcp_v4_rcv+0x4e9/0x500) from [<c015025d>] (ip_local_deliver_finish+0x79/0x180)
>> [27224.170000] [<c015025d>] (ip_local_deliver_finish+0x79/0x180) from [<c01500a1>] (ip_rcv_finish+0xc9/0x20c)
>> [27224.180000] [<c01500a1>] (ip_rcv_finish+0xc9/0x20c) from [<c0129493>] (__netif_receive_skb_core+0x287/0x3dc)
>> [27224.190000] [<c0129493>] (__netif_receive_skb_core+0x287/0x3dc) from [<c0129a8b>] (process_backlog+0x5b/0xd0)
>> [27224.200000] [<c0129a8b>] (process_backlog+0x5b/0xd0) from [<c012bc6f>] (net_rx_action+0x5f/0xe8)
>> [27224.200000] [<c012bc6f>] (net_rx_action+0x5f/0xe8) from [<c0015fc9>] (__do_softirq+0x91/0x134)
>> [27224.210000] [<c0015fc9>] (__do_softirq+0x91/0x134) from [<c0016251>] (irq_exit+0x59/0x80)
>> [27224.220000] [<c0016251>] (irq_exit+0x59/0x80) from [<c000d00d>] (handle_IRQ+0x21/0x54)
>> [27224.230000] [<c000d00d>] (handle_IRQ+0x21/0x54) from [<c00083ef>] (sun4i_handle_irq+0x1f/0x30)
>> [27224.240000] [<c00083ef>] (sun4i_handle_irq+0x1f/0x30) from [<c000ea1b>] (__irq_svc+0x3b/0x5c)
>> [27224.250000] Exception stack(0xc0287f70 to 0xc0287fb8)
>> [27224.250000] 7f60:                                     00000000 00000000 00000000 00000000
>> [27224.260000] 7f80: c0286000 c02ad7fd 00000001 c02ad7fd c028e084 413fc082 00000000 00000000
>> [27224.270000] 7fa0: 01000000 c0287fb8 c000d115 c000d116 40000033 ffffffff
>> [27224.270000] [<c000ea1b>] (__irq_svc+0x3b/0x5c) from [<c000d116>] (arch_cpu_idle+0x16/0x1c)
>> [27224.280000] [<c000d116>] (arch_cpu_idle+0x16/0x1c) from [<c002e69d>] (cpu_startup_entry+0x35/0xa0)
>> [27224.290000] [<c002e69d>] (cpu_startup_entry+0x35/0xa0) from [<c026f75f>] (start_kernel+0x1af/0x1f0)
>> [27224.300000] ---[ end trace 15641276e08fba8a ]---
>
> Hmmm, that seems pretty far from the network/interrupt drivers, and it
> looks like other users have seen this on !ARM machines:
> http://forums.gentoo.org/viewtopic-p-7379928.html
> https://bugzilla.redhat.com/show_bug.cgi?id=989251
>
> There's a patch in the redhat's bugzilla, could you try to apply it and
> see if it solves your problem?
Thanks for those links !
I'll give it a try.

>
> Maybe the netdev guys will have other ideas as well.
>
> Thanks,
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com

Thanks !

-- 
for me, ck means con kolivas and not calvin klein... does it mean I'm a geek ?

^ permalink raw reply

* [PATCH] bridge: separate querier and query timer into IGMP/IPv4 and MLD/IPv6 ones
From: Linus Lüssing @ 2013-08-26 10:01 UTC (permalink / raw)
  To: bridge
  Cc: Stephen Hemminger, David S. Miller, netdev, linux-kernel,
	Herbert Xu, Cong Wang, Adam Baker, Linus Lüssing

Currently we would still potentially suffer multicast packet loss if there
is just either an IGMP or an MLD querier: For the former case, we would
possibly drop IPv6 multicast packets, for the latter IPv4 ones. This is
because we are currently assuming that if either an IGMP or MLD querier
is present that the other one is present, too.

This patch makes the behaviour and fix added in
"bridge: disable snooping if there is no querier" (b00589af3b04)
to also work if there is either just an IGMP or an MLD querier on the
link: It refines the deactivation of the snooping to be protocol
specific by using separate timers for the snooped IGMP and MLD queries
as well as separate timers for our internal IGMP and MLD queriers.

Signed-off-by: Linus Lüssing <linus.luessing@web.de>
---
 net/bridge/br_device.c    |    2 +-
 net/bridge/br_input.c     |    2 +-
 net/bridge/br_mdb.c       |   14 +-
 net/bridge/br_multicast.c |  338 +++++++++++++++++++++++++++++++++++----------
 net/bridge/br_private.h   |   47 +++++--
 5 files changed, 313 insertions(+), 90 deletions(-)

diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index 69363bd..89659d4 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -71,7 +71,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
 
 		mdst = br_mdb_get(br, skb, vid);
 		if ((mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) &&
-		    br_multicast_querier_exists(br))
+		    br_multicast_querier_exists(br, eth_hdr(skb)))
 			br_multicast_deliver(mdst, skb);
 		else
 			br_flood_deliver(br, skb, false);
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 8c561c0..a2fd37e 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -102,7 +102,7 @@ int br_handle_frame_finish(struct sk_buff *skb)
 	} else if (is_multicast_ether_addr(dest)) {
 		mdst = br_mdb_get(br, skb, vid);
 		if ((mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) &&
-		    br_multicast_querier_exists(br)) {
+		    br_multicast_querier_exists(br, eth_hdr(skb))) {
 			if ((mdst && mdst->mglist) ||
 			    br_multicast_is_router(br))
 				skb2 = skb;
diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
index 0daae3e..6053b96 100644
--- a/net/bridge/br_mdb.c
+++ b/net/bridge/br_mdb.c
@@ -414,16 +414,20 @@ static int __br_mdb_del(struct net_bridge *br, struct br_mdb_entry *entry)
 	if (!netif_running(br->dev) || br->multicast_disabled)
 		return -EINVAL;
 
-	if (timer_pending(&br->multicast_querier_timer))
-		return -EBUSY;
-
 	ip.proto = entry->addr.proto;
-	if (ip.proto == htons(ETH_P_IP))
+	if (ip.proto == htons(ETH_P_IP)) {
+		if (timer_pending(&br->ip4_multicast_querier_timer))
+			return -EBUSY;
+
 		ip.u.ip4 = entry->addr.u.ip4;
 #if IS_ENABLED(CONFIG_IPV6)
-	else
+	} else {
+		if (timer_pending(&br->ip6_multicast_querier_timer))
+			return -EBUSY;
+
 		ip.u.ip6 = entry->addr.u.ip6;
 #endif
+	}
 
 	spin_lock_bh(&br->multicast_lock);
 	mdb = mlock_dereference(br->mdb, br);
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 08e576a..e1c01f4 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -33,7 +33,10 @@
 
 #include "br_private.h"
 
-static void br_multicast_start_querier(struct net_bridge *br);
+static void br_ip4_multicast_start_querier(struct net_bridge *br);
+#if IS_ENABLED(CONFIG_IPV6)
+static void br_ip6_multicast_start_querier(struct net_bridge *br);
+#endif
 unsigned int br_mdb_rehash_seq;
 
 static inline int br_ip_equal(const struct br_ip *a, const struct br_ip *b)
@@ -755,7 +758,22 @@ static void br_multicast_local_router_expired(unsigned long data)
 {
 }
 
-static void br_multicast_querier_expired(unsigned long data)
+static void br_ip4_multicast_querier_expired(unsigned long data)
+{
+	struct net_bridge *br = (void *)data;
+
+	spin_lock(&br->multicast_lock);
+	if (!netif_running(br->dev) || br->multicast_disabled)
+		goto out;
+
+	br_ip4_multicast_start_querier(br);
+
+out:
+	spin_unlock(&br->multicast_lock);
+}
+
+#if IS_ENABLED(CONFIG_IPV6)
+static void br_ip6_multicast_querier_expired(unsigned long data)
 {
 	struct net_bridge *br = (void *)data;
 
@@ -763,11 +781,12 @@ static void br_multicast_querier_expired(unsigned long data)
 	if (!netif_running(br->dev) || br->multicast_disabled)
 		goto out;
 
-	br_multicast_start_querier(br);
+	br_ip6_multicast_start_querier(br);
 
 out:
 	spin_unlock(&br->multicast_lock);
 }
+#endif
 
 static void __br_multicast_send_query(struct net_bridge *br,
 				      struct net_bridge_port *port,
@@ -788,15 +807,15 @@ static void __br_multicast_send_query(struct net_bridge *br,
 		netif_rx(skb);
 }
 
-static void br_multicast_send_query(struct net_bridge *br,
-				    struct net_bridge_port *port, u32 sent)
+static void br_ip4_multicast_send_query(struct net_bridge *br,
+					struct net_bridge_port *port, u32 sent)
 {
 	unsigned long time;
 	struct br_ip br_group;
 
 	if (!netif_running(br->dev) || br->multicast_disabled ||
 	    !br->multicast_querier ||
-	    timer_pending(&br->multicast_querier_timer))
+	    timer_pending(&br->ip4_multicast_querier_timer))
 		return;
 
 	memset(&br_group.u, 0, sizeof(br_group.u));
@@ -804,20 +823,41 @@ static void br_multicast_send_query(struct net_bridge *br,
 	br_group.proto = htons(ETH_P_IP);
 	__br_multicast_send_query(br, port, &br_group);
 
+	time = jiffies;
+	time += sent < br->multicast_startup_query_count ?
+		br->multicast_startup_query_interval :
+		br->multicast_query_interval;
+	mod_timer(port ? &port->ip4_multicast_query_timer :
+			 &br->ip4_multicast_query_timer, time);
+}
+
 #if IS_ENABLED(CONFIG_IPV6)
+static void br_ip6_multicast_send_query(struct net_bridge *br,
+					struct net_bridge_port *port, u32 sent)
+{
+	unsigned long time;
+	struct br_ip br_group;
+
+	if (!netif_running(br->dev) || br->multicast_disabled ||
+	    !br->multicast_querier ||
+	    timer_pending(&br->ip6_multicast_querier_timer))
+		return;
+
+	memset(&br_group.u, 0, sizeof(br_group.u));
+
 	br_group.proto = htons(ETH_P_IPV6);
 	__br_multicast_send_query(br, port, &br_group);
-#endif
 
 	time = jiffies;
 	time += sent < br->multicast_startup_query_count ?
 		br->multicast_startup_query_interval :
 		br->multicast_query_interval;
-	mod_timer(port ? &port->multicast_query_timer :
-			 &br->multicast_query_timer, time);
+	mod_timer(port ? &port->ip6_multicast_query_timer :
+			 &br->ip6_multicast_query_timer, time);
 }
+#endif
 
-static void br_multicast_port_query_expired(unsigned long data)
+static void br_ip4_multicast_port_query_expired(unsigned long data)
 {
 	struct net_bridge_port *port = (void *)data;
 	struct net_bridge *br = port->br;
@@ -827,25 +867,52 @@ static void br_multicast_port_query_expired(unsigned long data)
 	    port->state == BR_STATE_BLOCKING)
 		goto out;
 
-	if (port->multicast_startup_queries_sent <
+	if (port->ip4_multicast_startup_queries_sent <
 	    br->multicast_startup_query_count)
-		port->multicast_startup_queries_sent++;
+		port->ip4_multicast_startup_queries_sent++;
 
-	br_multicast_send_query(port->br, port,
-				port->multicast_startup_queries_sent);
+	br_ip4_multicast_send_query(port->br, port,
+				    port->ip4_multicast_startup_queries_sent);
 
 out:
 	spin_unlock(&br->multicast_lock);
 }
 
+#if IS_ENABLED(CONFIG_IPV6)
+static void br_ip6_multicast_port_query_expired(unsigned long data)
+{
+	struct net_bridge_port *port = (void *)data;
+	struct net_bridge *br = port->br;
+
+	spin_lock(&br->multicast_lock);
+	if (port->state == BR_STATE_DISABLED ||
+	    port->state == BR_STATE_BLOCKING)
+		goto out;
+
+	if (port->ip6_multicast_startup_queries_sent <
+	    br->multicast_startup_query_count)
+		port->ip6_multicast_startup_queries_sent++;
+
+	br_ip6_multicast_send_query(port->br, port,
+				    port->ip6_multicast_startup_queries_sent);
+
+out:
+	spin_unlock(&br->multicast_lock);
+}
+#endif
+
 void br_multicast_add_port(struct net_bridge_port *port)
 {
 	port->multicast_router = 1;
 
 	setup_timer(&port->multicast_router_timer, br_multicast_router_expired,
 		    (unsigned long)port);
-	setup_timer(&port->multicast_query_timer,
-		    br_multicast_port_query_expired, (unsigned long)port);
+	setup_timer(&port->ip4_multicast_query_timer,
+		    br_ip4_multicast_port_query_expired, (unsigned long)port);
+#if IS_ENABLED(CONFIG_IPV6)
+	setup_timer(&port->ip6_multicast_query_timer,
+		    br_ip6_multicast_port_query_expired, (unsigned long)port);
+#endif
 }
 
 void br_multicast_del_port(struct net_bridge_port *port)
@@ -853,14 +920,25 @@ void br_multicast_del_port(struct net_bridge_port *port)
 	del_timer_sync(&port->multicast_router_timer);
 }
 
-static void __br_multicast_enable_port(struct net_bridge_port *port)
+static void __br_ip4_multicast_enable_port(struct net_bridge_port *port)
+{
+	port->ip4_multicast_startup_queries_sent = 0;
+
+	if (try_to_del_timer_sync(&port->ip4_multicast_query_timer) >= 0 ||
+	    del_timer(&port->ip4_multicast_query_timer))
+		mod_timer(&port->ip4_multicast_query_timer, jiffies);
+}
+
+#if IS_ENABLED(CONFIG_IPV6)
+static void __br_ip6_multicast_enable_port(struct net_bridge_port *port)
 {
-	port->multicast_startup_queries_sent = 0;
+	port->ip6_multicast_startup_queries_sent = 0;
 
-	if (try_to_del_timer_sync(&port->multicast_query_timer) >= 0 ||
-	    del_timer(&port->multicast_query_timer))
-		mod_timer(&port->multicast_query_timer, jiffies);
+	if (try_to_del_timer_sync(&port->ip6_multicast_query_timer) >= 0 ||
+	    del_timer(&port->ip6_multicast_query_timer))
+		mod_timer(&port->ip6_multicast_query_timer, jiffies);
 }
+#endif
 
 void br_multicast_enable_port(struct net_bridge_port *port)
 {
@@ -870,7 +948,10 @@ void br_multicast_enable_port(struct net_bridge_port *port)
 	if (br->multicast_disabled || !netif_running(br->dev))
 		goto out;
 
-	__br_multicast_enable_port(port);
+	__br_ip4_multicast_enable_port(port);
+#if IS_ENABLED(CONFIG_IPV6)
+	__br_ip6_multicast_enable_port(port);
+#endif
 
 out:
 	spin_unlock(&br->multicast_lock);
@@ -889,7 +970,8 @@ void br_multicast_disable_port(struct net_bridge_port *port)
 	if (!hlist_unhashed(&port->rlist))
 		hlist_del_init_rcu(&port->rlist);
 	del_timer(&port->multicast_router_timer);
-	del_timer(&port->multicast_query_timer);
+	del_timer(&port->ip4_multicast_query_timer);
+	del_timer(&port->ip6_multicast_query_timer);
 	spin_unlock(&br->multicast_lock);
 }
 
@@ -1014,16 +1096,28 @@ static int br_ip6_multicast_mld2_report(struct net_bridge *br,
 }
 #endif
 
-static void br_multicast_update_querier_timer(struct net_bridge *br,
-					      unsigned long max_delay)
+static void br_ip4_multicast_update_querier_timer(struct net_bridge *br,
+						  unsigned long max_delay)
 {
-	if (!timer_pending(&br->multicast_querier_timer))
-		br->multicast_querier_delay_time = jiffies + max_delay;
+	if (!timer_pending(&br->ip4_multicast_querier_timer))
+		br->ip4_multicast_querier_delay_time = jiffies + max_delay;
 
-	mod_timer(&br->multicast_querier_timer,
+	mod_timer(&br->ip4_multicast_querier_timer,
 		  jiffies + br->multicast_querier_interval);
 }
 
+#if IS_ENABLED(CONFIG_IPV6)
+static void br_ip6_multicast_update_querier_timer(struct net_bridge *br,
+						  unsigned long max_delay)
+{
+	if (!timer_pending(&br->ip6_multicast_querier_timer))
+		br->ip6_multicast_querier_delay_time = jiffies + max_delay;
+
+	mod_timer(&br->ip6_multicast_querier_timer,
+		  jiffies + br->multicast_querier_interval);
+}
+#endif
+
 /*
  * Add port to router_list
  *  list is maintained ordered by pointer value
@@ -1072,18 +1166,33 @@ timer:
 		  now + br->multicast_querier_interval);
 }
 
-static void br_multicast_query_received(struct net_bridge *br,
-					struct net_bridge_port *port,
-					int saddr,
-					unsigned long max_delay)
+static void br_ip4_multicast_query_received(struct net_bridge *br,
+					    struct net_bridge_port *port,
+					    int saddr,
+					    unsigned long max_delay)
+{
+	if (saddr)
+		br_ip4_multicast_update_querier_timer(br, max_delay);
+	else if (timer_pending(&br->ip4_multicast_querier_timer))
+		return;
+
+	br_multicast_mark_router(br, port);
+}
+
+#if IS_ENABLED(CONFIG_IPV6)
+static void br_ip6_multicast_query_received(struct net_bridge *br,
+					    struct net_bridge_port *port,
+					    int saddr,
+					    unsigned long max_delay)
 {
 	if (saddr)
-		br_multicast_update_querier_timer(br, max_delay);
-	else if (timer_pending(&br->multicast_querier_timer))
+		br_ip6_multicast_update_querier_timer(br, max_delay);
+	else if (timer_pending(&br->ip6_multicast_querier_timer))
 		return;
 
 	br_multicast_mark_router(br, port);
 }
+#endif
 
 static int br_ip4_multicast_query(struct net_bridge *br,
 				  struct net_bridge_port *port,
@@ -1129,7 +1238,7 @@ static int br_ip4_multicast_query(struct net_bridge *br,
 			    IGMPV3_MRC(ih3->code) * (HZ / IGMP_TIMER_SCALE) : 1;
 	}
 
-	br_multicast_query_received(br, port, !!iph->saddr, max_delay);
+	br_ip4_multicast_query_received(br, port, !!iph->saddr, max_delay);
 
 	if (!group)
 		goto out;
@@ -1206,8 +1315,8 @@ static int br_ip6_multicast_query(struct net_bridge *br,
 		max_delay = mld2q->mld2q_mrc ? MLDV2_MRC(ntohs(mld2q->mld2q_mrc)) : 1;
 	}
 
-	br_multicast_query_received(br, port, !ipv6_addr_any(&ip6h->saddr),
-				    max_delay);
+	br_ip6_multicast_query_received(br, port, !ipv6_addr_any(&ip6h->saddr),
+					max_delay);
 
 	if (!group)
 		goto out;
@@ -1252,25 +1361,23 @@ static void br_multicast_leave_group(struct net_bridge *br,
 	unsigned long now;
 	unsigned long time;
 
-	spin_lock(&br->multicast_lock);
-	if (!netif_running(br->dev) ||
-	    (port && port->state == BR_STATE_DISABLED) ||
-	    timer_pending(&br->multicast_querier_timer))
-		goto out;
-
 	mdb = mlock_dereference(br->mdb, br);
 	mp = br_mdb_ip_get(mdb, group);
 	if (!mp)
-		goto out;
+		return;
 
-	if (br->multicast_querier &&
-	    !timer_pending(&br->multicast_querier_timer)) {
+	if (br->multicast_querier) {
 		__br_multicast_send_query(br, port, &mp->addr);
 
 		time = jiffies + br->multicast_last_member_count *
 				 br->multicast_last_member_interval;
-		mod_timer(port ? &port->multicast_query_timer :
-				 &br->multicast_query_timer, time);
+
+		if (group->proto == htons(ETH_P_IP))
+			mod_timer(port ? &port->ip4_multicast_query_timer :
+				  &br->ip4_multicast_query_timer, time);
+		else
+			mod_timer(port ? &port->ip6_multicast_query_timer :
+				  &br->ip6_multicast_query_timer, time);
 
 		for (p = mlock_dereference(mp->ports, br);
 		     p != NULL;
@@ -1308,7 +1415,7 @@ static void br_multicast_leave_group(struct net_bridge *br,
 			    netif_running(br->dev))
 				mod_timer(&mp->timer, jiffies);
 		}
-		goto out;
+		return;
 	}
 
 	now = jiffies;
@@ -1323,9 +1430,6 @@ static void br_multicast_leave_group(struct net_bridge *br,
 			mod_timer(&mp->timer, time);
 		}
 	}
-
-out:
-	spin_unlock(&br->multicast_lock);
 }
 
 static void br_ip4_multicast_leave_group(struct net_bridge *br,
@@ -1342,7 +1446,15 @@ static void br_ip4_multicast_leave_group(struct net_bridge *br,
 	br_group.proto = htons(ETH_P_IP);
 	br_group.vid = vid;
 
+	spin_lock(&br->multicast_lock);
+	if (!netif_running(br->dev) ||
+	    (port && port->state == BR_STATE_DISABLED) ||
+	    timer_pending(&br->ip4_multicast_querier_timer))
+		goto out;
+
 	br_multicast_leave_group(br, port, &br_group);
+out:
+	spin_unlock(&br->multicast_lock);
 }
 
 #if IS_ENABLED(CONFIG_IPV6)
@@ -1360,7 +1472,15 @@ static void br_ip6_multicast_leave_group(struct net_bridge *br,
 	br_group.proto = htons(ETH_P_IPV6);
 	br_group.vid = vid;
 
+	spin_lock(&br->multicast_lock);
+	if (!netif_running(br->dev) ||
+	    (port && port->state == BR_STATE_DISABLED) ||
+	    timer_pending(&br->ip6_multicast_querier_timer))
+		goto out;
+
 	br_multicast_leave_group(br, port, &br_group);
+out:
+	spin_unlock(&br->multicast_lock);
 }
 #endif
 
@@ -1622,20 +1742,38 @@ int br_multicast_rcv(struct net_bridge *br, struct net_bridge_port *port,
 	return 0;
 }
 
-static void br_multicast_query_expired(unsigned long data)
+static void br_ip4_multicast_query_expired(unsigned long data)
 {
 	struct net_bridge *br = (void *)data;
 
 	spin_lock(&br->multicast_lock);
-	if (br->multicast_startup_queries_sent <
+	if (br->ip4_multicast_startup_queries_sent <
 	    br->multicast_startup_query_count)
-		br->multicast_startup_queries_sent++;
+		br->ip4_multicast_startup_queries_sent++;
 
-	br_multicast_send_query(br, NULL, br->multicast_startup_queries_sent);
+	br_ip4_multicast_send_query(br, NULL,
+				    br->ip4_multicast_startup_queries_sent);
 
 	spin_unlock(&br->multicast_lock);
 }
 
+#if IS_ENABLED(CONFIG_IPV6)
+static void br_ip6_multicast_query_expired(unsigned long data)
+{
+	struct net_bridge *br = (void *)data;
+
+	spin_lock(&br->multicast_lock);
+	if (br->ip6_multicast_startup_queries_sent <
+	    br->multicast_startup_query_count)
+		br->ip6_multicast_startup_queries_sent++;
+
+	br_ip6_multicast_send_query(br, NULL,
+				    br->ip6_multicast_startup_queries_sent);
+
+	spin_unlock(&br->multicast_lock);
+}
+#endif
+
 void br_multicast_init(struct net_bridge *br)
 {
 	br->hash_elasticity = 4;
@@ -1654,25 +1792,52 @@ void br_multicast_init(struct net_bridge *br)
 	br->multicast_querier_interval = 255 * HZ;
 	br->multicast_membership_interval = 260 * HZ;
 
-	br->multicast_querier_delay_time = 0;
+	br->ip4_multicast_querier_delay_time = 0;
+	br->ip6_multicast_querier_delay_time = 0;
 
 	spin_lock_init(&br->multicast_lock);
 	setup_timer(&br->multicast_router_timer,
 		    br_multicast_local_router_expired, 0);
-	setup_timer(&br->multicast_querier_timer,
-		    br_multicast_querier_expired, (unsigned long)br);
-	setup_timer(&br->multicast_query_timer, br_multicast_query_expired,
-		    (unsigned long)br);
+	setup_timer(&br->ip4_multicast_querier_timer,
+		    br_ip4_multicast_querier_expired, (unsigned long)br);
+	setup_timer(&br->ip4_multicast_query_timer,
+		    br_ip4_multicast_query_expired, (unsigned long)br);
+#if IS_ENABLED(CONFIG_IPV6)
+	setup_timer(&br->ip6_multicast_querier_timer,
+		    br_ip6_multicast_querier_expired, (unsigned long)br);
+	setup_timer(&br->ip6_multicast_query_timer,
+		    br_ip6_multicast_query_expired, (unsigned long)br);
+#endif
 }
 
-void br_multicast_open(struct net_bridge *br)
+static void br_ip4_multicast_open(struct net_bridge *br)
 {
-	br->multicast_startup_queries_sent = 0;
+	br->ip4_multicast_startup_queries_sent = 0;
 
 	if (br->multicast_disabled)
 		return;
 
-	mod_timer(&br->multicast_query_timer, jiffies);
+	mod_timer(&br->ip4_multicast_query_timer, jiffies);
+}
+
+#if IS_ENABLED(CONFIG_IPV6)
+static void br_ip6_multicast_open(struct net_bridge *br)
+{
+	br->ip6_multicast_startup_queries_sent = 0;
+
+	if (br->multicast_disabled)
+		return;
+
+	mod_timer(&br->ip6_multicast_query_timer, jiffies);
+}
+#endif
+
+void br_multicast_open(struct net_bridge *br)
+{
+	br_ip4_multicast_open(br);
+#if IS_ENABLED(CONFIG_IPV6)
+	br_ip6_multicast_open(br);
+#endif
 }
 
 void br_multicast_stop(struct net_bridge *br)
@@ -1684,8 +1849,10 @@ void br_multicast_stop(struct net_bridge *br)
 	int i;
 
 	del_timer_sync(&br->multicast_router_timer);
-	del_timer_sync(&br->multicast_querier_timer);
-	del_timer_sync(&br->multicast_query_timer);
+	del_timer_sync(&br->ip4_multicast_querier_timer);
+	del_timer_sync(&br->ip6_multicast_querier_timer);
+	del_timer_sync(&br->ip4_multicast_query_timer);
+	del_timer_sync(&br->ip6_multicast_query_timer);
 
 	spin_lock_bh(&br->multicast_lock);
 	mdb = mlock_dereference(br->mdb, br);
@@ -1788,21 +1955,38 @@ unlock:
 	return err;
 }
 
-static void br_multicast_start_querier(struct net_bridge *br)
+static void br_ip4_multicast_start_querier(struct net_bridge *br)
 {
 	struct net_bridge_port *port;
 
-	br_multicast_open(br);
+	br_ip4_multicast_open(br);
 
 	list_for_each_entry(port, &br->port_list, list) {
 		if (port->state == BR_STATE_DISABLED ||
 		    port->state == BR_STATE_BLOCKING)
 			continue;
 
-		__br_multicast_enable_port(port);
+		__br_ip4_multicast_enable_port(port);
 	}
 }
 
+#if IS_ENABLED(CONFIG_IPV6)
+static void br_ip6_multicast_start_querier(struct net_bridge *br)
+{
+	struct net_bridge_port *port;
+
+	br_ip6_multicast_open(br);
+
+	list_for_each_entry(port, &br->port_list, list) {
+		if (port->state == BR_STATE_DISABLED ||
+		    port->state == BR_STATE_BLOCKING)
+			continue;
+
+		__br_ip6_multicast_enable_port(port);
+	}
+}
+#endif
+
 int br_multicast_toggle(struct net_bridge *br, unsigned long val)
 {
 	int err = 0;
@@ -1834,7 +2018,10 @@ rollback:
 			goto rollback;
 	}
 
-	br_multicast_start_querier(br);
+	br_ip4_multicast_start_querier(br);
+#if IS_ENABLED(CONFIG_IPV6)
+	br_ip6_multicast_start_querier(br);
+#endif
 
 unlock:
 	spin_unlock_bh(&br->multicast_lock);
@@ -1857,10 +2044,15 @@ int br_multicast_set_querier(struct net_bridge *br, unsigned long val)
 		goto unlock;
 
 	max_delay = br->multicast_query_response_interval;
-	if (!timer_pending(&br->multicast_querier_timer))
-		br->multicast_querier_delay_time = jiffies + max_delay;
+	if (!timer_pending(&br->ip4_multicast_querier_timer))
+		br->ip4_multicast_querier_delay_time = jiffies + max_delay;
+	if (!timer_pending(&br->ip6_multicast_querier_timer))
+		br->ip6_multicast_querier_delay_time = jiffies + max_delay;
 
-	br_multicast_start_querier(br);
+	br_ip4_multicast_start_querier(br);
+#if IS_ENABLED(CONFIG_IPV6)
+	br_ip6_multicast_start_querier(br);
+#endif
 
 unlock:
 	spin_unlock_bh(&br->multicast_lock);
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 2f7da41..10b4231 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -162,10 +162,12 @@ struct net_bridge_port
 #define BR_FLOOD		0x00000040
 
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
-	u32				multicast_startup_queries_sent;
+	u32				ip4_multicast_startup_queries_sent;
+	u32				ip6_multicast_startup_queries_sent;
 	unsigned char			multicast_router;
 	struct timer_list		multicast_router_timer;
-	struct timer_list		multicast_query_timer;
+	struct timer_list		ip4_multicast_query_timer;
+	struct timer_list		ip6_multicast_query_timer;
 	struct hlist_head		mglist;
 	struct hlist_node		rlist;
 #endif
@@ -258,7 +260,8 @@ struct net_bridge
 	u32				hash_max;
 
 	u32				multicast_last_member_count;
-	u32				multicast_startup_queries_sent;
+	u32				ip4_multicast_startup_queries_sent;
+	u32				ip6_multicast_startup_queries_sent;
 	u32				multicast_startup_query_count;
 
 	unsigned long			multicast_last_member_interval;
@@ -267,15 +270,18 @@ struct net_bridge
 	unsigned long			multicast_query_interval;
 	unsigned long			multicast_query_response_interval;
 	unsigned long			multicast_startup_query_interval;
-	unsigned long			multicast_querier_delay_time;
+	unsigned long			ip4_multicast_querier_delay_time;
+	unsigned long			ip6_multicast_querier_delay_time;
 
 	spinlock_t			multicast_lock;
 	struct net_bridge_mdb_htable __rcu *mdb;
 	struct hlist_head		router_list;
 
 	struct timer_list		multicast_router_timer;
-	struct timer_list		multicast_querier_timer;
-	struct timer_list		multicast_query_timer;
+	struct timer_list		ip4_multicast_querier_timer;
+	struct timer_list		ip6_multicast_querier_timer;
+	struct timer_list		ip4_multicast_query_timer;
+	struct timer_list		ip6_multicast_query_timer;
 #endif
 
 	struct timer_list		hello_timer;
@@ -503,11 +509,31 @@ static inline bool br_multicast_is_router(struct net_bridge *br)
 		timer_pending(&br->multicast_router_timer));
 }
 
-static inline bool br_multicast_querier_exists(struct net_bridge *br)
+static inline bool br_ip4_multicast_querier_exists(struct net_bridge *br)
 {
-	return time_is_before_jiffies(br->multicast_querier_delay_time) &&
+	return time_is_before_jiffies(br->ip4_multicast_querier_delay_time) &&
 	       (br->multicast_querier ||
-		timer_pending(&br->multicast_querier_timer));
+		timer_pending(&br->ip4_multicast_querier_timer));
+}
+
+static inline bool br_ip6_multicast_querier_exists(struct net_bridge *br)
+{
+	return time_is_before_jiffies(br->ip6_multicast_querier_delay_time) &&
+	       (br->multicast_querier ||
+		timer_pending(&br->ip6_multicast_querier_timer));
+}
+
+static inline bool br_multicast_querier_exists(struct net_bridge *br,
+					       struct ethhdr *eth)
+{
+	switch (eth->h_proto) {
+	case (htons(ETH_P_IP)):
+		return br_ip4_multicast_querier_exists(br);
+	case (htons(ETH_P_IPV6)):
+		return br_ip6_multicast_querier_exists(br);
+	default:
+		return false;
+	}
 }
 #else
 static inline int br_multicast_rcv(struct net_bridge *br,
@@ -565,7 +591,8 @@ static inline bool br_multicast_is_router(struct net_bridge *br)
 {
 	return 0;
 }
-static inline bool br_multicast_querier_exists(struct net_bridge *br)
+static inline bool br_multicast_querier_exists(struct net_bridge *br,
+					       struct ethhdr *eth)
 {
 	return false;
 }
-- 
1.7.10.4

^ permalink raw reply related

* Re: Problematic commits in the ipsec tree
From: Steffen Klassert @ 2013-08-26  9:41 UTC (permalink / raw)
  To: Hannes Frederic Sowa; +Cc: David Miller, netdev
In-Reply-To: <20130823124911.GD808@order.stressinduktion.org>

On Fri, Aug 23, 2013 at 02:49:11PM +0200, Hannes Frederic Sowa wrote:
> 
> This could be the replacement for patch 1/2 to reassemble old behaviour
> without touching ip6_skb_dst_mtu if the socket type is not an IPv6 one.
> 
> I would still like to look if we could correctly handle *_PMTUDISC_PROBE one
> day and fallback to dst_mtu(dst->path) if possible. So I don't know if
> removing xfrm_skb_dst_mtu is good style and would just make churn in the git
> history. What do you think?

Currently I think we can call dst_mtu() unconditionally from
__xfrm6_output(), then we would not need xfrm_skb_dst_mtu().
But this needs further investigation, IPsec pmtu discovery
was frequently broken in the past and I don't want to break
it again.

> 
> [PATCH ipsec 1/2] xfrm: revert ipv4 mtu determination to dst_mtu
> 
> In commit 0ea9d5e3e0e03a63b11392f5613378977dae7eca ("xfrm: introduce
> helper for safe determination of mtu") I switched the determination of
> ipv4 mtus from dst_mtu to ip_skb_dst_mtu. This was an error because in
> case of IP_PMTUDISC_PROBE we fall back to the interface mtu, which is
> never correct for ipv4 ipsec.
> 
> This patch partly reverts 0ea9d5e3e0e03a63b11392f5613378977dae7eca
> ("xfrm: introduce helper for safe determination of mtu").
> 

I think with this and you other patch, we get the all the
interfamily tunnel problems fixed for now. Everything else
should be done in ipsec-next.

Please resend the whole patchset, so we can get it fixed soon.

Tanks a lot!

^ permalink raw reply

* Re: Do you guys think add link test to offline test is reasonable?
From: Ben Hutchings @ 2013-08-26  9:17 UTC (permalink / raw)
  To: Hui-Zhi Zhao; +Cc: netdev
In-Reply-To: <CAOhdcN_QPtfQK87LCmoS2k=7_W9Wzwke21PdvZUi9znZ3Si54Q@mail.gmail.com>

On Mon, 2013-08-26 at 15:47 +0800, Hui-Zhi Zhao wrote:
> Hi Guys,
> 
> 
> I have a query about the ethtool offline test.
> 
> 
> From the man page of "ethtool" I see the offline test should be
> "offline (default) means to perform full set of tests possibly causing
> normal operation interruption during the tests". In my opinion, it
> should just do something like inner loopback tests, offline should be
> no relative with live network connection.

I agree that an offline test should not depend on having a network link.

> Since it is impractical to expect these ports to be connected to a
> live network during production test (due to  limited network
> resources, a machine maybe have lots of ports), some of the tests will
> fail, even the cards have no problem.
>
> So, do you guys think add link test to offline test is reasonable?

You can do it but it shouldn't affect the overall pass/fail result.

Ben.

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

^ permalink raw reply

* Re: [PATCH] net/cadence/macb: fix kernel Oops if no PHY were discovered during probe
From: Bo Shen @ 2013-08-26  9:09 UTC (permalink / raw)
  To: Boris BREZILLON; +Cc: Nicolas Ferre, netdev, linux-kernel, linux-arm-kernel
In-Reply-To: <1377372065-8938-1-git-send-email-b.brezillon@overkiz.com>

Hi Boris,

On 08/25/2013 03:21 AM, Boris BREZILLON wrote:
> Test the presence of a PHY device before printing attached PHY
> informations.
>
> Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
> ---
>   drivers/net/ethernet/cadence/macb.c |    6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index e866608..fd3b67f 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -1868,8 +1868,10 @@ static int __init macb_probe(struct platform_device *pdev)
>   		    dev->irq, dev->dev_addr);
>
>   	phydev = bp->phy_dev;
> -	netdev_info(dev, "attached PHY driver [%s] (mii_bus:phy_addr=%s, irq=%d)\n",
> -		    phydev->drv->name, dev_name(&phydev->dev), phydev->irq);
> +	if (phydev)
> +		netdev_info(dev, "attached PHY driver [%s] (mii_bus:phy_addr=%s, irq=%d)\n",
> +			    phydev->drv->name, dev_name(&phydev->dev),
> +			    phydev->irq);

Actually no need this check, if PHY is attached failed, the macb driver 
probe will fail, then it won't show this message.

>   	return 0;
>
>

Best Regards,
Bo Shen

^ permalink raw reply

* Re: [PATCH v6 2/5] net: ethernet: cpsw: add optional third memory region for CONTROL module
From: Tony Lindgren @ 2013-08-26  9:04 UTC (permalink / raw)
  To: Daniel Mack
  Cc: netdev, bcousson, nsekhar, sergei.shtylyov, davem, ujhelyi.m,
	mugunthanvnm, vaibhav.bedia, d-gerlach, linux-arm-kernel,
	linux-omap, devicetree
In-Reply-To: <1377286330-29663-3-git-send-email-zonque@gmail.com>

* Daniel Mack <zonque@gmail.com> [130823 12:39]:
> At least the AM33xx SoC has a control module register to configure
> details such as the hardware ethernet interface mode.
> 
> I'm not sure whether all SoCs which feature the cpsw block have such a
> register, so that third memory region is considered optional for now.

Assuming you're talking about omap SCM registers here..

This should be in a separate driver module so the control module
parts can eventually be children of the SCM driver as they are
really separate devices on the bus. See how the USB PHY parts were done
for example.

What do these control module registers do? If it's just multiplexing
and pinconf, then you can use pinctrl-single,bits most likely for it and
access it using the named modes.

However, if the register also contains comparators and control for
regulators, you should only use pinctrl-single for the multiplexing
and pinconf parts.

Regards,

Tony

^ permalink raw reply

* Re: [PATCH] net/cadence/macb: fix kernel Oops if no PHY were discovered during probe
From: Nicolas Ferre @ 2013-08-26  7:43 UTC (permalink / raw)
  To: Boris BREZILLON; +Cc: netdev, linux-kernel, linux-arm-kernel
In-Reply-To: <1377372065-8938-1-git-send-email-b.brezillon@overkiz.com>

On 24/08/2013 21:21, Boris BREZILLON :
> Test the presence of a PHY device before printing attached PHY
> informations.
>
> Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

> ---
>   drivers/net/ethernet/cadence/macb.c |    6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index e866608..fd3b67f 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -1868,8 +1868,10 @@ static int __init macb_probe(struct platform_device *pdev)
>   		    dev->irq, dev->dev_addr);
>
>   	phydev = bp->phy_dev;
> -	netdev_info(dev, "attached PHY driver [%s] (mii_bus:phy_addr=%s, irq=%d)\n",
> -		    phydev->drv->name, dev_name(&phydev->dev), phydev->irq);
> +	if (phydev)
> +		netdev_info(dev, "attached PHY driver [%s] (mii_bus:phy_addr=%s, irq=%d)\n",
> +			    phydev->drv->name, dev_name(&phydev->dev),
> +			    phydev->irq);
>
>   	return 0;
>
>


-- 
Nicolas Ferre

^ permalink raw reply

* Re: [PATCH 6/6] vhost_net: remove the max pending check
From: Jason Wang @ 2013-08-26  7:00 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20130825115344.GB1829@redhat.com>

On 08/25/2013 07:53 PM, Michael S. Tsirkin wrote:
> On Fri, Aug 23, 2013 at 04:55:49PM +0800, Jason Wang wrote:
>> On 08/20/2013 10:48 AM, Jason Wang wrote:
>>> On 08/16/2013 06:02 PM, Michael S. Tsirkin wrote:
>>>>> On Fri, Aug 16, 2013 at 01:16:30PM +0800, Jason Wang wrote:
>>>>>>> We used to limit the max pending DMAs to prevent guest from pinning too many
>>>>>>> pages. But this could be removed since:
>>>>>>>
>>>>>>> - We have the sk_wmem_alloc check in both tun/macvtap to do the same work
>>>>>>> - This max pending check were almost useless since it was one done when there's
>>>>>>>   no new buffers coming from guest. Guest can easily exceeds the limitation.
>>>>>>> - We've already check upend_idx != done_idx and switch to non zerocopy then. So
>>>>>>>   even if all vq->heads were used, we can still does the packet transmission.
>>>>> We can but performance will suffer.
>>> The check were in fact only done when no new buffers submitted from
>>> guest. So if guest keep sending, the check won't be done.
>>>
>>> If we really want to do this, we should do it unconditionally. Anyway, I
>>> will do test to see the result.
>> There's a bug in PATCH 5/6, the check:
>>
>> nvq->upend_idx != nvq->done_idx
>>
>> makes the zerocopy always been disabled since we initialize both
>> upend_idx and done_idx to zero. So I change it to:
>>
>> (nvq->upend_idx + 1) % UIO_MAXIOV != nvq->done_idx.
> But what I would really like to try is limit ubuf_info to VHOST_MAX_PEND.
> I think this has a chance to improve performance since
> we'll be using less cache.

Maybe, but it in fact decrease the vq size to VHOST_MAX_PEND.
> Of course this means we must fix the code to really never submit
> more than VHOST_MAX_PEND requests.
>
> Want to try?

Ok, sure.
>> With this change on top, I didn't see performance difference w/ and w/o
>> this patch.
> Did you try small message sizes btw (like 1K)? Or just netperf
> default of 64K?
>

I just test multiple sessions of TCP_RR. Will test TCP_STREAM also.

^ permalink raw reply

* Re: [PATCH v4 3/5] net: ethernet: cpsw: introduce ti,am3352-cpsw compatible string
From: Sekhar Nori @ 2013-08-26  6:45 UTC (permalink / raw)
  To: Mugunthan V N
  Cc: Santosh Shilimkar, Daniel Mack, netdev, bcousson, sergei.shtylyov,
	davem, ujhelyi.m, vaibhav.bedia, d-gerlach, linux-arm-kernel,
	linux-omap, devicetree, Grant Likely, Rob Herring
In-Reply-To: <521AEECF.8060606@ti.com>

On Monday 26 August 2013 11:29 AM, Mugunthan V N wrote:
> On Saturday 24 August 2013 01:24 AM, Santosh Shilimkar wrote:
>> On Friday 23 August 2013 02:29 PM, Mugunthan V N wrote:
>>> On Friday 23 August 2013 11:40 PM, Santosh Shilimkar wrote:
>>>> On Friday 23 August 2013 01:39 PM, Sekhar Nori wrote:
>>>>> On 8/23/2013 10:58 PM, Santosh Shilimkar wrote:
>>>>>> On Friday 23 August 2013 01:24 PM, Daniel Mack wrote:
>>>>>>> On 23.08.2013 19:19, Santosh Shilimkar wrote:
>>>>>>>> On Friday 23 August 2013 01:09 PM, Sekhar Nori wrote:
>>>>>>>>> On 8/23/2013 10:26 PM, Santosh Shilimkar wrote:
>>>>>>>>>> So just stick the IP version or call it cpsw-v1... cpsw-v2 etc.
>>>>>>>>> If this could be handled using IP version then the right way would be to
>>>>>>>>> just read the IP version from hardware and use it. No need of DT property.
>>>>>>>>>
>>>>>>>> Thats fine as well but I thought the patch needed additional properties like
>>>>>>>> CM reg-address come from DT and hence the separate compatible. If you can
>>>>>>>> manage without that, thats even better.
>>>>>>> We can't, that's the whole point :)
>>>>>>>
>>>>>> I saw that from the patch :)
>>>>>>
>>>>>>> Well, theoretically, we could for now, but that's not a clean solution.
>>>>>>> Again: the problem here is that the control port is separated from the
>>>>>>> cpsw core, and so we have to implement something specific for the AM3352
>>>>>>> SoC. I know that's a violation of clean and generic driver ideas, but
>>>>>>> there's no way we can assume that every cpsw v2 ip block has a control
>>>>>>> port that is compatible to the one found on am335x chips.
>>>>>>>
>>>>>> But there is a possibility that other SOC will just use the same
>>>>>> control module approach. So using a revision IP is just fine. BTW,
>>>>> But this is misleading because it makes appear like the same compatible
>>>>> can be used on on another SoC like DRA7 which probably has the same
>>>>> version of IP but a different control module implementation, when in
>>>>> practice it cannot.
>>>>>
>>>>> The fact is we are doing something SoC specific in the driver and we
>>>>> cannot hide that behind IP versions. If really in practice there comes
>>>>> another SoC with the same control module definition then it can always
>>>>> use ti,am3352-cpsw compatible as well. The compatible name does not
>>>>> preclude its usage.
>>>>>
>>>> My point was the CPSW needs a feature which is implemented using
>>>> control module rather than within the IP itself. Its an implementation
>>>> detail. As such the additional feature makes sense for that IP. O.w
>>>> there was no need to do any monkeying with control module.
>>>>
>>>> E.g
>>>> MMC card detect is a basic functionality, implemented by various types
>>>> like control module, PMIC or MMC IP itself. As such the driver need
>>>> that support and all the implementation details needs to still handled
>>>> to make that part work.
>>>>
>>>>
>>> CPSW core as such understands only GMII/MII signals, there is an
>>> additional module which converts GMII/MII signals to RGMII/RMII signals
>>> respectively which is called as CPRGMII/CPRMII as specified in the
>>> AM335x TRM in Figure 14-1. Ethernet Switch Integration.
>>>
>>> So to control this sub-module, the control register is used and this has
>>> to be configured according to the EVM design like what mode of phy is
>>> connected. CPRGMII and CPRMII is no way related to CPSW core.
>>>
>> Ok then why are you polluting cpsw driver with that code which
>> not realted to CPSW as you said above. You are contradicting what
>> you said by supporting the SOC usage in the core CPSW driver.
> This patch series is not from me and because of the reason I mentioned
> about control module driver, so that cpsw driver can make use control
> module apis to select phy mode and control module driver takes care of
> SoC specific register offsets and definitions, but now it is not
> possible as it is not acceptable in mainline. So other way is to keep
> these in driver itself as it is done in this patch series with SoC
> compatibilities.

What is done in this patch is _not_ "SoC compatibilities". SoC
compatibility would be what was done in v1 of this patch ie, explicit
check for

	of_machine_is_compatible("ti,am33xx")

"ti,am3352-cpsw" says "CPSW as implemented on AM3352". This is not the
same as checking if SoC is AM3352.

The example quoted on ePAPR spec for a compatible string is:

	compatible = “fsl,mpc8641-uart”, “ns16550";

MPC8641 is freescale PowerPC based SoC[1]. This shows that it is not
unnatural to use SoC names in compatibles for IPs. That, or the ePAPR
specification needs to be updated to show the right example of how a
compatible could be defined. Until then I see no reason of changing what
is implemented in this patch.

In short, even if there was no control module handling in the driver,
using "ti,am3352-cpsw" would be just fine.

I have also CCed the DT maintainers for their opinion. They should have
been explicitly CCed anyway.

Thanks,
Sekhar

[1] http://www.freescale.com/files/32bit/doc/data_sheet/MPC8641DEC.pdf

^ permalink raw reply

* Re: [PATCH 1/2] genl: Fix genl dumpit() locking.
From: Johannes Berg @ 2013-08-26  6:06 UTC (permalink / raw)
  To: Pravin Shelar; +Cc: netdev, Jesse Gross
In-Reply-To: <CALnjE+qn8THcPABZojWoE3OW2Y2TKZGSjB24ZWLC5AGM5yKm9w@mail.gmail.com>

On Fri, 2013-08-23 at 13:52 -0700, Pravin Shelar wrote:

> > I'm still missing something. Kernel 3.4 had cb_mutex assign to the
> > genl_mutex, but we saw the original crash there, apparently dumpit
> > *wasn't* (always) locked with it?

> Can you point me to original crash on 3.4?

Sure, below.

johannes


[1389854.965295] cfg80211: Calling CRDA to update world regulatory domain
[1389854.973801] Intel(R) Wireless WiFi driver for Linux, in-tree:d
[1389854.973804] Copyright(c) 2003-2013 Intel Corporation
[1389854.982900] cfg80211: World regulatory domain updated:
[1389854.982908] cfg80211:   (start_freq - end_freq @ bandwidth), (max_antenna_gain, max_eirp)
[1389854.982913] cfg80211:   (2402000 KHz - 2472000 KHz @ 40000 KHz), (300 mBi, 2000 mBm)
[1389854.982919] cfg80211:   (2457000 KHz - 2482000 KHz @ 20000 KHz), (300 mBi, 2000 mBm)
[1389854.982923] cfg80211:   (2474000 KHz - 2494000 KHz @ 20000 KHz), (300 mBi, 2000 mBm)
[1389854.982928] cfg80211:   (5170000 KHz - 5250000 KHz @ 40000 KHz), (300 mBi, 2000 mBm)
[1389854.982932] cfg80211:   (5735000 KHz - 5835000 KHz @ 40000 KHz), (300 mBi, 2000 mBm)
[1389857.247719] BUG: unable to handle kernel paging request at f8467360
[1389857.249716] IP: [<c14c56bb>] ctrl_dumpfamily+0x6b/0xe0
[1389857.251798] *pde = 2ffd7067 *pte = 00000000
[1389857.253903] Oops: 0000 [#1] SMP DEBUG_PAGEALLOC
[1389857.256002] Modules linked in: cfg80211(O) ...
[1389857.265729]
[1389857.268159] Pid: 20081, comm: wpa_supplicant Tainted: G        W  O 3.4.47-dev #1 Dell Inc. Latitude E6430/0CPWYR
[1389857.270726] EIP: 0060:[<c14c56bb>] EFLAGS: 00210297 CPU: 2
[1389857.273291] EIP is at ctrl_dumpfamily+0x6b/0xe0
[1389857.275829] EAX: f8467378 EBX: f8467340 ECX: 00000000 EDX: ec1610c4
[1389857.278365] ESI: 00000001 EDI: c2077cc0 EBP: c46c3c00 ESP: c46c3bd4
[1389857.280921]  DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
[1389857.283508] CR0: 80050033 CR2: f8467360 CR3: 26e54000 CR4: 001407d0
[1389857.286130] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
[1389857.288770] DR6: ffff0ff0 DR7: 00000400
[1389857.291363] Process wpa_supplicant (pid: 20081, ti=c46c2000 task=c44640b0 task.ti=c46c2000)
[1389857.294044] Stack:
[1389857.296668]  00000002 caef8000 00000001 caef8000 00000000 e6ccc3c0 c1861f00 00000000
[1389857.299377]  e73cd910 e6ccc3c0 caef8000 c46c3c28 c14c20bc 000000d0 00200246 00200246
[1389857.302077]  e73cd910 e6ccc3c0 e73cd910 e6ccc3c0 00000000 c46c3c48 c14c3450 d0757b00
[1389857.304794] Call Trace:
[1389857.307443]  [<c14c20bc>] netlink_dump+0x5c/0x200
[1389857.310110]  [<c14c3450>] __netlink_dump_start+0x140/0x160
[1389857.312779]  [<c14c5650>] ? ctrl_fill_info+0x370/0x370
[1389857.315442]  [<c14c5172>] genl_rcv_msg+0x102/0x270
[1389857.318096]  [<c14c5034>] ? genl_lock+0x14/0x20
[1389857.320765]  [<c15acdb2>] ? mutex_lock_nested+0x222/0x2f0
[1389857.323424]  [<c15acdc2>] ? mutex_lock_nested+0x232/0x2f0
[1389857.326026]  [<c14c5034>] ? genl_lock+0x14/0x20
[1389857.328621]  [<c14c5650>] ? ctrl_fill_info+0x370/0x370
[1389857.331224]  [<c14c5070>] ? genl_rcv+0x30/0x30
[1389857.333822]  [<c14c4b5e>] netlink_rcv_skb+0x8e/0xb0
[1389857.336420]  [<c14c505c>] genl_rcv+0x1c/0x30
[1389857.339014]  [<c14c456b>] netlink_unicast+0x17b/0x1c0
[1389857.341617]  [<c14c47d4>] netlink_sendmsg+0x224/0x370
[1389857.344215]  [<c1485adf>] sock_sendmsg+0xff/0x120
[1389857.346812]  [<c112ad04>] ? might_fault+0x54/0xb0
[1389857.349403]  [<c112ad4e>] ? might_fault+0x9e/0xb0
[1389857.351982]  [<c12de3c2>] ? _copy_from_user+0x42/0x60
[1389857.354558]  [<c14926e4>] ? verify_iovec+0x44/0xb0
[1389857.357086]  [<c1486b0a>] __sys_sendmsg+0x24a/0x260
[1389857.359563]  [<c12e39de>] ? do_raw_spin_unlock+0x4e/0x90
[1389857.362025]  [<c110af35>] ? unlock_page+0x45/0x50
[1389857.364443]  [<c112aff8>] ? __do_fault+0x298/0x450
[1389857.366858]  [<c112db01>] ? handle_pte_fault+0xe1/0x7d0
[1389857.369178]  [<c15b3b8b>] ? do_page_fault+0xcb/0x4b0
[1389857.371403]  [<c1150b45>] ? fget_light+0x1d5/0x470
[1389857.373618]  [<c1487fdb>] sys_sendmsg+0x3b/0x60
[1389857.375827]  [<c1488683>] sys_socketcall+0x283/0x2e0
[1389857.377946]  [<c15b072d>] ? restore_all+0xf/0xf
[1389857.379981]  [<c15b3ac0>] ? vmalloc_fault+0x114/0x114
[1389857.381927]  [<c12ddea8>] ? trace_hardirqs_on_thunk+0xc/0x10
[1389857.383796]  [<c15b7c1f>] sysenter_do_call+0x12/0x38
[1389857.385644] Code: 8d 3c c5 c0 7c 07 c2 8b 04 c5 c0 7c 07 c2 39 c7 8d 58 c8 75 16 eb 71 90 81 7d ec 00 1f 86 c1 74 10 8b 43 38 39 c7 8d 58 c8 74 5d <80> 7b 20 00 74 e7 83 c6 01 3b 75 f0 7c e8 8b 55 e8 8b 42 04 8b
[1389857.389897] EIP: [<c14c56bb>] ctrl_dumpfamily+0x6b/0xe0 SS:ESP 0068:c46c3bd4
[1389857.391949] CR2: 00000000f8467360
[1389857.496970] ---[ end trace 52efe903d218886a ]---
[1389857.496977] BUG: sleeping function called from invalid context at kernel/rwsem.c:20
[1389857.496982] in_atomic(): 0, irqs_disabled(): 1, pid: 20081, name: wpa_supplicant
[1389857.496986] INFO: lockdep is turned off.
[1389857.496989] irq event stamp: 0
[1389857.496992] hardirqs last  enabled at (0): [<  (null)>]   (null)
[1389857.496997] hardirqs last disabled at (0): [<c1031aa8>] copy_process+0x468/0x1280
[1389857.497006] softirqs last  enabled at (0): [<c1031aa8>] copy_process+0x468/0x1280
[1389857.497012] softirqs last disabled at (0): [<  (null)>]   (null)
[1389857.497019] Pid: 20081, comm: wpa_supplicant Tainted: G      D W  O 3.4.47-dev #1
[1389857.497022] Call Trace:
[1389857.497031]  [<c1067322>] __might_sleep+0x162/0x200
[1389857.497038]  [<c15add80>] down_read+0x20/0x8b
[1389857.497046]  [<c1049f5e>] exit_signals+0x1e/0x110
[1389857.497053]  [<c10381b7>] do_exit+0x97/0x9b0
[1389857.497059]  [<c1035753>] ? kmsg_dump+0x193/0x270
[1389857.497065]  [<c1035630>] ? kmsg_dump+0x70/0x270
[1389857.497073]  [<c15a60e2>] ? printk+0x2d/0x2f
[1389857.497079]  [<c15b1646>] oops_end+0x96/0xd0
[1389857.497086]  [<c15a5aac>] no_context+0x18c/0x194
[1389857.497098]  [<c15a5bf8>] __bad_area_nosemaphore+0x144/0x14c
[1389857.497106]  [<c10960bb>] ? trace_hardirqs_on+0xb/0x10
[1389857.497114]  [<c148c7cf>] ? sock_rmalloc+0x3f/0x90
[1389857.497122]  [<c15b3ac0>] ? vmalloc_fault+0x114/0x114
[1389857.497128]  [<c15a5c17>] bad_area_nosemaphore+0x17/0x19
[1389857.497135]  [<c15b3d9f>] do_page_fault+0x2df/0x4b0
[1389857.497141]  [<c14c2049>] ? __nlmsg_put+0x59/0x70
[1389857.497149]  [<c12ec362>] ? __nla_reserve+0x42/0x60
[1389857.497154]  [<c15b0e54>] ? error_code+0x68/0x74
[1389857.497160]  [<c15b3ac0>] ? vmalloc_fault+0x114/0x114
[1389857.497167]  [<c1093adf>] ? trace_hardirqs_off_caller+0x1f/0x130
[1389857.497176]  [<c15b3ac0>] ? vmalloc_fault+0x114/0x114
[1389857.497181]  [<c15b0e58>] error_code+0x6c/0x74
[1389857.497191]  [<c14c56bb>] ? ctrl_dumpfamily+0x6b/0xe0
[1389857.497197]  [<c14c20bc>] netlink_dump+0x5c/0x200
[1389857.497204]  [<c14c3450>] __netlink_dump_start+0x140/0x160
[1389857.497210]  [<c14c5650>] ? ctrl_fill_info+0x370/0x370
[1389857.497216]  [<c14c5172>] genl_rcv_msg+0x102/0x270
[1389857.497222]  [<c14c5034>] ? genl_lock+0x14/0x20
[1389857.497229]  [<c15acdb2>] ? mutex_lock_nested+0x222/0x2f0
[1389857.497236]  [<c15acdc2>] ? mutex_lock_nested+0x232/0x2f0
[1389857.497242]  [<c14c5034>] ? genl_lock+0x14/0x20
[1389857.497248]  [<c14c5650>] ? ctrl_fill_info+0x370/0x370
[1389857.497254]  [<c14c5070>] ? genl_rcv+0x30/0x30
[1389857.497260]  [<c14c4b5e>] netlink_rcv_skb+0x8e/0xb0
[1389857.497267]  [<c14c505c>] genl_rcv+0x1c/0x30
[1389857.497273]  [<c14c456b>] netlink_unicast+0x17b/0x1c0
[1389857.497279]  [<c14c47d4>] netlink_sendmsg+0x224/0x370
[1389857.497286]  [<c1485adf>] sock_sendmsg+0xff/0x120
[1389857.497294]  [<c112ad04>] ? might_fault+0x54/0xb0
[1389857.497301]  [<c112ad4e>] ? might_fault+0x9e/0xb0
[1389857.497308]  [<c12de3c2>] ? _copy_from_user+0x42/0x60
[1389857.497313]  [<c14926e4>] ? verify_iovec+0x44/0xb0
[1389857.497320]  [<c1486b0a>] __sys_sendmsg+0x24a/0x260
[1389857.497326]  [<c12e39de>] ? do_raw_spin_unlock+0x4e/0x90
[1389857.497333]  [<c110af35>] ? unlock_page+0x45/0x50
[1389857.497340]  [<c112aff8>] ? __do_fault+0x298/0x450
[1389857.497346]  [<c112db01>] ? handle_pte_fault+0xe1/0x7d0
[1389857.497353]  [<c15b3b8b>] ? do_page_fault+0xcb/0x4b0
[1389857.497359]  [<c1150b45>] ? fget_light+0x1d5/0x470
[1389857.497366]  [<c1487fdb>] sys_sendmsg+0x3b/0x60
[1389857.497372]  [<c1488683>] sys_socketcall+0x283/0x2e0
[1389857.497378]  [<c15b072d>] ? restore_all+0xf/0xf
[1389857.497384]  [<c15b3ac0>] ? vmalloc_fault+0x114/0x114
[1389857.497391]  [<c12ddea8>] ? trace_hardirqs_on_thunk+0xc/0x10
[1389857.497397]  [<c15b7c1f>] sysenter_do_call+0x12/0x38

^ permalink raw reply

* Re: [PATCH RFC net-next] net: epoll support for busy poll
From: Eliezer Tamir @ 2013-08-26  6:03 UTC (permalink / raw)
  To: Amir Vadai
  Cc: Eliezer Tamir, Willem de Bruijn, Eilon Greenstein, Eric Wong,
	netdev, e1000-devel, Eric Dumazet, linux-kernel, David Miller
In-Reply-To: <CAPcc5PhQGB195pMsMGOEE8BhZTvfXOf+Fh+3PdPbuOS0t1SULQ@mail.gmail.com>

On 26/08/2013 00:30, Amir Vadai wrote:
> I'm on vacation, will test and have some inputs later this week when I be
> back.
> 
Hello Amir,

Ping me when you get back and I will send you my latest so you can
play with it.

^ permalink raw reply

* Re: [PATCH v4 3/5] net: ethernet: cpsw: introduce ti,am3352-cpsw compatible string
From: Mugunthan V N @ 2013-08-26  5:59 UTC (permalink / raw)
  To: Santosh Shilimkar
  Cc: devicetree, sergei.shtylyov, d-gerlach, netdev, Sekhar Nori,
	vaibhav.bedia, Daniel Mack, bcousson, ujhelyi.m, linux-omap,
	davem, linux-arm-kernel
In-Reply-To: <5217BDFA.5040904@ti.com>

On Saturday 24 August 2013 01:24 AM, Santosh Shilimkar wrote:
> On Friday 23 August 2013 02:29 PM, Mugunthan V N wrote:
>> On Friday 23 August 2013 11:40 PM, Santosh Shilimkar wrote:
>>> On Friday 23 August 2013 01:39 PM, Sekhar Nori wrote:
>>>> On 8/23/2013 10:58 PM, Santosh Shilimkar wrote:
>>>>> On Friday 23 August 2013 01:24 PM, Daniel Mack wrote:
>>>>>> On 23.08.2013 19:19, Santosh Shilimkar wrote:
>>>>>>> On Friday 23 August 2013 01:09 PM, Sekhar Nori wrote:
>>>>>>>> On 8/23/2013 10:26 PM, Santosh Shilimkar wrote:
>>>>>>>>> So just stick the IP version or call it cpsw-v1... cpsw-v2 etc.
>>>>>>>> If this could be handled using IP version then the right way would be to
>>>>>>>> just read the IP version from hardware and use it. No need of DT property.
>>>>>>>>
>>>>>>> Thats fine as well but I thought the patch needed additional properties like
>>>>>>> CM reg-address come from DT and hence the separate compatible. If you can
>>>>>>> manage without that, thats even better.
>>>>>> We can't, that's the whole point :)
>>>>>>
>>>>> I saw that from the patch :)
>>>>>
>>>>>> Well, theoretically, we could for now, but that's not a clean solution.
>>>>>> Again: the problem here is that the control port is separated from the
>>>>>> cpsw core, and so we have to implement something specific for the AM3352
>>>>>> SoC. I know that's a violation of clean and generic driver ideas, but
>>>>>> there's no way we can assume that every cpsw v2 ip block has a control
>>>>>> port that is compatible to the one found on am335x chips.
>>>>>>
>>>>> But there is a possibility that other SOC will just use the same
>>>>> control module approach. So using a revision IP is just fine. BTW,
>>>> But this is misleading because it makes appear like the same compatible
>>>> can be used on on another SoC like DRA7 which probably has the same
>>>> version of IP but a different control module implementation, when in
>>>> practice it cannot.
>>>>
>>>> The fact is we are doing something SoC specific in the driver and we
>>>> cannot hide that behind IP versions. If really in practice there comes
>>>> another SoC with the same control module definition then it can always
>>>> use ti,am3352-cpsw compatible as well. The compatible name does not
>>>> preclude its usage.
>>>>
>>> My point was the CPSW needs a feature which is implemented using
>>> control module rather than within the IP itself. Its an implementation
>>> detail. As such the additional feature makes sense for that IP. O.w
>>> there was no need to do any monkeying with control module.
>>>
>>> E.g
>>> MMC card detect is a basic functionality, implemented by various types
>>> like control module, PMIC or MMC IP itself. As such the driver need
>>> that support and all the implementation details needs to still handled
>>> to make that part work.
>>>
>>>
>> CPSW core as such understands only GMII/MII signals, there is an
>> additional module which converts GMII/MII signals to RGMII/RMII signals
>> respectively which is called as CPRGMII/CPRMII as specified in the
>> AM335x TRM in Figure 14-1. Ethernet Switch Integration.
>>
>> So to control this sub-module, the control register is used and this has
>> to be configured according to the EVM design like what mode of phy is
>> connected. CPRGMII and CPRMII is no way related to CPSW core.
>>
> Ok then why are you polluting cpsw driver with that code which
> not realted to CPSW as you said above. You are contradicting what
> you said by supporting the SOC usage in the core CPSW driver.
This patch series is not from me and because of the reason I mentioned
about control module driver, so that cpsw driver can make use control
module apis to select phy mode and control module driver takes care of
SoC specific register offsets and definitions, but now it is not
possible as it is not acceptable in mainline. So other way is to keep
these in driver itself as it is done in this patch series with SoC
compatibilities.

Regards
Mugunthan V N

^ permalink raw reply

* Re: [GLIBC Patch v2] inet: avoid redefinition of some structs in kernel
From: Mike Frysinger @ 2013-08-26  5:26 UTC (permalink / raw)
  To: libc-alpha
  Cc: Cong Wang, netdev, David S. Miller, Thomas Backlund,
	YOSHIFUJI Hideaki, Carlos O'Donell
In-Reply-To: <1376558891-26221-2-git-send-email-amwang@redhat.com>

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

On Thursday 15 August 2013 05:28:11 Cong Wang wrote:
> From: Carlos O'Donell <carlos@redhat.com>
> 
> - Synchronize linux's `include/uapi/linux/in6.h'
>   with glibc's `inet/netinet/in.h'.
> - Synchronize glibc's `inet/netinet/in.h with linux's
>   `include/uapi/linux/in6.h'.
> - Allow including the headers in either other.
> - First header included defines the structures and macros.
> 
> Notes:
> - You want netinet/in.h to include bits/in.h as early as possible,
>   but it needs in_addr so define in_addr early.
> - You want bits/in.h included as early as possible so you can use
>   the linux specific code to define __USE_KERNEL_DEFS based on
>   the _UAPI_* macro definition and use those to cull in.h.
> - glibc was missing IPPROTO_MH, added here.

can we get something better documented here in a central location ?  having 
the names laid out in a git commit message and in a few leaf headers does not 
lend itself to being easily discoverable.
-mike

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

^ permalink raw reply

* RE: [PATCH v4 3/5] net: ethernet: cpsw: introduce ti,am3352-cpsw compatible string
From: Gupta, Pekon @ 2013-08-26  5:22 UTC (permalink / raw)
  To: N, Mugunthan V, Shilimkar, Santosh, bcousson@baylibre.com,
	Daniel Mack, Nori, Sekhar
  Cc: devicetree@vger.kernel.org, sergei.shtylyov@cogentembedded.com,
	Gerlach, Dave, netdev@vger.kernel.org, Bedia, Vaibhav,
	ujhelyi.m@gmail.com, linux-omap@vger.kernel.org,
	davem@davemloft.net, linux-arm-kernel@lists.infradead.org
In-Reply-To: <52179AA5.3060408@ti.com>

> 
> On Friday 23 August 2013 10:26 PM, Santosh Shilimkar wrote:
> > On Friday 23 August 2013 12:30 PM, Daniel Mack wrote:
> >> On 23.08.2013 16:23, Santosh Shilimkar wrote:
> >>> On Friday 23 August 2013 10:16 AM, Daniel Mack wrote:
> >>>> +static const struct of_device_id cpsw_of_mtable[] = {
> >>>> +	{
> >>>> +		.compatible	= "ti,am3352-cpsw",
> >>> I didn't notice this earlier, but can't you use the IP version
> >>> as a compatible instead of using a SOC name. Whats really SOC specific
> >>> on this IP ? Sorry i have missed any earlier discussion on this but
> >>> this approach doesn't seem good. Its like adding SOC checks in the
> >>> driver subsystem.
> >> As I already mentioned in the cover letter and in the commit message, I
> >> just don't know which criteria makes most sense here.
> >>
> >> On a general note, I would say that chances that this exactly IP core
> >> with the same version number will appear on some other silicon which
> >> doesn't support the control mode register in an AM33xx fashion, is not
> >> necessarily negligible.
> >>
> >> So what that new compatible string denotes is the cpsw in a version as
> >> found on am3352 SoCs, which is actually exactly what it does.
> >>
> >> I don't have a strong opinion here, but see your point. I just don't
> >> have a better idea on how to treat that.
> >>
> > So just stick the IP version or call it cpsw-v1... cpsw-v2 etc.
> > That way if in future if someone uses those features, they can use
> > this compatible if they don't they use the one which suites that
> > SOC.
> >
> We cannot map control module register with CPSW IP version as both comes
> from different design team and CPSW ip version can be same across SoC
> and gmii sel register definition can be different. Control module
> defines may vary in different SoC as per SoC requirements.
> 
> Adding Pekon Gupta who had worked in Silicon team before.
> 
[Pekon]: My opinion here..
Uniformity in control module cannot be guaranteed, neither in register
offsets nor their functionality. Uniformity is usually maintained till the
point same person is writing the spec, or it’s a derivative device.
Control-module should not be categorized as IP, instead it’s a group of
miscellaneous logic usually consisting of following:
(a) SoC bug-fixes across silicon revisions.
(b) SoC specific logic like device_type, JTAG-ID.
(c) IP bug fixes which could not be accommodated in IP address-map.
(d) IP logic which depends on SoC configurations.

Due to this un-deterministic composition of control-module, having 
a dedicated driver for control module might not work either, as it
has to be updated | re-written for every new device.

So, Following can be used as guideline to determine compatibility
string for DT bindings..
- If binding maps to (a) and (c), .i.e., bug-fixes for SoC or IP, 
 then its most likely that these would change either in next silicon
 revision or in next devices. Hence such bindings should use *not*
 use IP compatibility strings, they _may_ use SoC-name based
 compatibility string.

- Else if binding maps to (b),.i.e., SoC specific configurations.
 then their offsets and functionality should remain same across the
 family of devices at-least, so it should use SoC-name based
 compatibility string.
- And if binding maps to (d), .i.e., IP feature but depending on SoC
 Then it can use IP based compatibility string, along with IP version.

(same approach can be used for CPSW binding used here,

with regards, pekon
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH v2 net-next] tcp: TSO packets automatic sizing
From: Eric Dumazet @ 2013-08-26  4:26 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, Neal Cardwell, Yuchung Cheng, Van Jacobson, Tom Herbert
In-Reply-To: <1377304192.8828.43.camel@edumazet-glaptop>

From: Eric Dumazet <edumazet@google.com>

After hearing many people over past years complaining against TSO being
bursty or even buggy, we are proud to present automatic sizing of TSO
packets.

One part of the problem is that tcp_tso_should_defer() uses an heuristic
relying on upcoming ACKS instead of a timer, but more generally, having
big TSO packets makes little sense for low rates, as it tends to create
micro bursts on the network, and general consensus is to reduce the
buffering amount.

This patch introduces a per socket sk_pacing_rate, that approximates
the current sending rate, and allows us to size the TSO packets so
that we try to send one packet every ms.

This field could be set by other transports.

Patch has no impact for high speed flows, where having large TSO packets
makes sense to reach line rate.

For other flows, this helps better packet scheduling and ACK clocking.

This patch increases performance of TCP flows in lossy environments.

A new sysctl (tcp_min_tso_segs) is added, to specify the
minimal size of a TSO packet (default being 2).

A follow-up patch will provide a new packet scheduler (FQ), using
sk_pacing_rate as an input to perform optional per flow pacing.

This explains why we chose to set sk_pacing_rate to twice the current
rate, allowing 'slow start' ramp up.

sk_pacing_rate = 2 * cwnd * mss / srtt
 
v2: Neal Cardwell reported a suspect deferring of last two segments on
initial write of 10 MSS, I had to change tcp_tso_should_defer() to take
into account tp->xmit_size_goal_segs 

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Neal Cardwell <ncardwell@google.com>
Cc: Yuchung Cheng <ycheng@google.com>
Cc: Van Jacobson <vanj@google.com>
Cc: Tom Herbert <therbert@google.com>
---
 Documentation/networking/ip-sysctl.txt |    9 ++++++
 include/net/sock.h                     |    2 +
 include/net/tcp.h                      |    1 
 net/ipv4/sysctl_net_ipv4.c             |   10 +++++++
 net/ipv4/tcp.c                         |   28 +++++++++++++++++---
 net/ipv4/tcp_input.c                   |   31 ++++++++++++++++++++++-
 net/ipv4/tcp_output.c                  |    2 -
 7 files changed, 76 insertions(+), 7 deletions(-)

diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index debfe85..ce5bb43 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -482,6 +482,15 @@ tcp_syn_retries - INTEGER
 tcp_timestamps - BOOLEAN
 	Enable timestamps as defined in RFC1323.
 
+tcp_min_tso_segs - INTEGER
+	Minimal number of segments per TSO frame.
+	Since linux-3.12, TCP does an automatic sizing of TSO frames,
+	depending on flow rate, instead of filling 64Kbytes packets.
+	For specific usages, it's possible to force TCP to build big
+	TSO frames. Note that TCP stack might split too big TSO packets
+	if available window is too small.
+	Default: 2
+
 tcp_tso_win_divisor - INTEGER
 	This allows control over what percentage of the congestion window
 	can be consumed by a single TSO frame.
diff --git a/include/net/sock.h b/include/net/sock.h
index e4bbcbf..6ba2e7b 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -232,6 +232,7 @@ struct cg_proto;
   *	@sk_napi_id: id of the last napi context to receive data for sk
   *	@sk_ll_usec: usecs to busypoll when there is no data
   *	@sk_allocation: allocation mode
+  *	@sk_pacing_rate: Pacing rate (if supported by transport/packet scheduler)
   *	@sk_sndbuf: size of send buffer in bytes
   *	@sk_flags: %SO_LINGER (l_onoff), %SO_BROADCAST, %SO_KEEPALIVE,
   *		   %SO_OOBINLINE settings, %SO_TIMESTAMPING settings
@@ -361,6 +362,7 @@ struct sock {
 	kmemcheck_bitfield_end(flags);
 	int			sk_wmem_queued;
 	gfp_t			sk_allocation;
+	u32			sk_pacing_rate; /* bytes per second */
 	netdev_features_t	sk_route_caps;
 	netdev_features_t	sk_route_nocaps;
 	int			sk_gso_type;
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 09cb5c1..73fcd7c 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -281,6 +281,7 @@ extern int sysctl_tcp_early_retrans;
 extern int sysctl_tcp_limit_output_bytes;
 extern int sysctl_tcp_challenge_ack_limit;
 extern unsigned int sysctl_tcp_notsent_lowat;
+extern int sysctl_tcp_min_tso_segs;
 
 extern atomic_long_t tcp_memory_allocated;
 extern struct percpu_counter tcp_sockets_allocated;
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 8ed7c32..540279f 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -29,6 +29,7 @@
 static int zero;
 static int one = 1;
 static int four = 4;
+static int gso_max_segs = GSO_MAX_SEGS;
 static int tcp_retr1_max = 255;
 static int ip_local_port_range_min[] = { 1, 1 };
 static int ip_local_port_range_max[] = { 65535, 65535 };
@@ -761,6 +762,15 @@ static struct ctl_table ipv4_table[] = {
 		.extra2		= &four,
 	},
 	{
+		.procname	= "tcp_min_tso_segs",
+		.data		= &sysctl_tcp_min_tso_segs,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= &zero,
+		.extra2		= &gso_max_segs,
+	},
+	{
 		.procname	= "udp_mem",
 		.data		= &sysctl_udp_mem,
 		.maxlen		= sizeof(sysctl_udp_mem),
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index ab64eea..e1714ee 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -283,6 +283,8 @@
 
 int sysctl_tcp_fin_timeout __read_mostly = TCP_FIN_TIMEOUT;
 
+int sysctl_tcp_min_tso_segs __read_mostly = 2;
+
 struct percpu_counter tcp_orphan_count;
 EXPORT_SYMBOL_GPL(tcp_orphan_count);
 
@@ -785,12 +787,28 @@ static unsigned int tcp_xmit_size_goal(struct sock *sk, u32 mss_now,
 	xmit_size_goal = mss_now;
 
 	if (large_allowed && sk_can_gso(sk)) {
-		xmit_size_goal = ((sk->sk_gso_max_size - 1) -
-				  inet_csk(sk)->icsk_af_ops->net_header_len -
-				  inet_csk(sk)->icsk_ext_hdr_len -
-				  tp->tcp_header_len);
+		u32 gso_size, hlen;
+
+		/* Maybe we should/could use sk->sk_prot->max_header here ? */
+		hlen = inet_csk(sk)->icsk_af_ops->net_header_len +
+		       inet_csk(sk)->icsk_ext_hdr_len +
+		       tp->tcp_header_len;
+
+		/* Goal is to send at least one packet per ms,
+		 * not one big TSO packet every 100 ms.
+		 * This preserves ACK clocking and is consistent
+		 * with tcp_tso_should_defer() heuristic.
+		 */
+		gso_size = sk->sk_pacing_rate / (2 * MSEC_PER_SEC);
+		gso_size = max_t(u32, gso_size,
+				 sysctl_tcp_min_tso_segs * mss_now);
+
+		xmit_size_goal = min_t(u32, gso_size,
+				       sk->sk_gso_max_size - 1 - hlen);
 
-		/* TSQ : try to have two TSO segments in flight */
+		/* TSQ : try to have at least two segments in flight
+		 * (one in NIC TX ring, another in Qdisc)
+		 */
 		xmit_size_goal = min_t(u32, xmit_size_goal,
 				       sysctl_tcp_limit_output_bytes >> 1);
 
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index ec492ea..3d63db7 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -688,6 +688,33 @@ static void tcp_rtt_estimator(struct sock *sk, const __u32 mrtt)
 	}
 }
 
+/* Set sk_pacing_rate to allow proper sizing of TSO packets.
+ * Note: TCP stack does not yet implement pacing.
+ * FQ packet scheduler can be used to implement cheap but effective
+ * TCP pacing, to smooth the burst on large writes when packets
+ * in flight is significantly lower than cwnd (or rwin)
+ */
+static void tcp_update_pacing_rate(struct sock *sk)
+{
+	const struct tcp_sock *tp = tcp_sk(sk);
+	u64 rate;
+
+	/* set sk_pacing_rate to 200 % of current rate (mss * cwnd / rtt) */
+	rate = (u64)tp->mss_cache * 8 * 2 * USEC_PER_SEC;
+
+	rate *= max(tp->snd_cwnd, tp->packets_out);
+
+	do_div(rate, jiffies_to_usecs(tp->srtt));
+
+	/* Correction for small srtt : minimum srtt being 8 (1 jiffy),
+	 * be conservative and assume rtt = 1/(8*HZ) instead of 1/HZ s
+	 * We probably need usec resolution in the future.
+	 */
+	if (tp->srtt <= 8 + 2)
+		rate <<= 3;
+	sk->sk_pacing_rate = min_t(u64, rate, ~0U);
+}
+
 /* Calculate rto without backoff.  This is the second half of Van Jacobson's
  * routine referred to above.
  */
@@ -3278,7 +3305,7 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
 	u32 ack_seq = TCP_SKB_CB(skb)->seq;
 	u32 ack = TCP_SKB_CB(skb)->ack_seq;
 	bool is_dupack = false;
-	u32 prior_in_flight;
+	u32 prior_in_flight, prior_cwnd = tp->snd_cwnd, prior_rtt = tp->srtt;
 	u32 prior_fackets;
 	int prior_packets = tp->packets_out;
 	const int prior_unsacked = tp->packets_out - tp->sacked_out;
@@ -3383,6 +3410,8 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
 
 	if (icsk->icsk_pending == ICSK_TIME_RETRANS)
 		tcp_schedule_loss_probe(sk);
+	if (tp->srtt != prior_rtt || tp->snd_cwnd != prior_cwnd)
+		tcp_update_pacing_rate(sk);
 	return 1;
 
 no_queue:
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 884efff..e63ae4c 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1631,7 +1631,7 @@ static bool tcp_tso_should_defer(struct sock *sk, struct sk_buff *skb)
 
 	/* If a full-sized TSO skb can be sent, do it. */
 	if (limit >= min_t(unsigned int, sk->sk_gso_max_size,
-			   sk->sk_gso_max_segs * tp->mss_cache))
+			   tp->xmit_size_goal_segs * tp->mss_cache))
 		goto send_now;
 
 	/* Middle in queue won't get any more data, full sendable already? */

^ permalink raw reply related

* Re: [PATCH net-next] tcp: TSO packets automatic sizing
From: Eric Dumazet @ 2013-08-26  3:58 UTC (permalink / raw)
  To: Yuchung Cheng
  Cc: Neal Cardwell, David Miller, netdev, Van Jacobson, Tom Herbert
In-Reply-To: <1377483759.8828.104.camel@edumazet-glaptop>

On Sun, 2013-08-25 at 19:22 -0700, Eric Dumazet wrote:
> On Sun, 2013-08-25 at 17:37 -0700, Eric Dumazet wrote:
> > On Sun, 2013-08-25 at 15:01 -0700, Yuchung Cheng wrote:
> > 
> > > Any idea to get rid of this undesirable extra RTT delay?
> > 
> > Its probably a bug in the push code.
> 
> For exact write/send of a multiple of MSS, I think following patch
> should fix the bug.
> 
> If we filled a packet, we must send it.
> 
> For the other problem, I think its related to Nagle.

Oh well, thats the tcp_tso_should_defer() again.

        /* If a full-sized TSO skb can be sent, do it. */
        if (limit >= min_t(unsigned int, sk->sk_gso_max_size,
                           sk->sk_gso_max_segs * tp->mss_cache))
                goto send_now;

I'll send a V2 of the patch

^ 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