Netdev List
 help / color / mirror / Atom feed
* Re: [RFC PATCH net-next] ipvs: add missing lock in ip_vs_ftp_init_conn()
From: Xiaotian Feng @ 2012-06-29  1:50 UTC (permalink / raw)
  To: Julian Anastasov
  Cc: netdev, lvs-devel, netfilter-devel, netfilter, coreteam,
	linux-kernel, Xiaotian Feng, Wensong Zhang, Simon Horman,
	Pablo Neira Ayuso, Patrick McHardy, David S. Miller
In-Reply-To: <alpine.LFD.2.00.1206290225490.1722@ja.ssi.bg>

On Fri, Jun 29, 2012 at 8:17 AM, Julian Anastasov <ja@ssi.bg> wrote:
>
>        Hello,
>
> On Thu, 28 Jun 2012, Xiaotian Feng wrote:
>
>> We met a kernel panic in 2.6.32.43 kernel:
>>
>> [2680191.848044] IPVS: ip_vs_conn_hash(): request for already hashed, called from run_timer_softirq+0x175/0x1d0
>> <snip>
>> [2680311.849009] general protection fault: 0000 [#1] SMP
>> [2680311.853001] RIP: 0010:[<ffffffff815f155c>]  [<ffffffff815f155c>] ip_vs_conn_expire+0xdc/0x2f0
>> [2680311.853001] RSP: 0018:ffff880028303e70  EFLAGS: 00010202
>> [2680311.853001] RAX: dead000000200200 RBX: ffff8801aad00b80 RCX: 0000000000001d90
>> [2680311.853001] RDX: dead000000100100 RSI: 000000004fd59800 RDI: ffff8801aad00c08
>> <snip>
>> [2680311.853001] Call Trace:
>> [2680311.853001]  <IRQ>
>> [2680311.853001]  [<ffffffff815f1480>] ? ip_vs_conn_expire+0x0/0x2f0
>> [2680311.853001]  [<ffffffff8104e2a5>] run_timer_softirq+0x175/0x1d0
>> [2680311.853001]  [<ffffffff81021a48>] ? lapic_next_event+0x18/0x20
>> [2680311.853001]  [<ffffffff81049a13>] __do_softirq+0xb3/0x150
>> [2680311.853001]  [<ffffffff8100cc5c>] call_softirq+0x1c/0x30
>> [2680311.853001]  [<ffffffff8100ea9a>] do_softirq+0x4a/0x80
>> [2680311.853001]  [<ffffffff81049957>] irq_exit+0x77/0x80
>> [2680311.853001]  [<ffffffff81021f2c>] smp_apic_timer_interrupt+0x6c/0xa0
>> [2680311.853001]  [<ffffffff8100c633>] apic_timer_interrupt+0x13/0x20
>> [2680311.853001]  <EOI>
>> [2680311.853001]  [<ffffffff81013b52>] ? mwait_idle+0x52/0x70
>> [2680311.853001]  [<ffffffff8100a7b0>] ? enter_idle+0x20/0x30
>> [2680311.853001]  [<ffffffff8100ac62>] ? cpu_idle+0x52/0x80
>> [2680311.853001]  [<ffffffff816d504d>] ? start_secondary+0x19d/0x280
>>
>> rax and rdx is LIST_POISON1 and LIST_POISON2, so kernel is list_del() on an already deleted
>> connection and result the general protect fault.
>>
>> The "request for already hashed" warning, told us someone might change the connection flags
>> incorrectly, like described in commit aea9d711, it changes the connection flags, but doesn't
>> put the connection back to the list. So ip_vs_conn_hash() throw a warning and return.
>> Later, when ip_vs_conn_expire fire again, ip_vs_conn_unhash() will find the HASHED connection
>> and list_del() it, then kernel panic happened.
>>
>> After code review, the only chance that kernel change connection flag without protection is
>> in ip_vs_ftp_init_conn().
>
>        Hm, ip_vs_ftp_init_conn is called before 1st hashing,
> from ip_vs_bind_app() in ip_vs_conn_new() before
> ip_vs_conn_hash(). It should be another problem with
> the flags. How different is IPVS in 2.6.32.43 compared to
> recent kernels? If commit aea9d711 is present, I'm not
> aware of other similar problems.

ip_vs_bind_app() is also called by ip_vs_try_bind_dest(), which can be
traced to ip_vs_proc_conn().
I've checked the changes in upstream, but nothing helps since aea9d711
has been taken into 2.6.32.28 kernel.

>
>        May be you can post some details for your setup on
> lvs-devel@vger.kernel.org. I assume FTP is used,
> what about master-backup sync? Can you confirm that
> this fix solves the problem or it happens in rare cases?

Yes, ftp and WRR scheduler is used. Unfortunately, the oops
is unreproducible :(

>
>> Signed-off-by: Xiaotian Feng <dannyfeng@tencent.com>
>> Cc: Wensong Zhang <wensong@linux-vs.org>
>> Cc: Simon Horman <horms@verge.net.au>
>> Cc: Julian Anastasov <ja@ssi.bg>
>> Cc: Pablo Neira Ayuso <pablo@netfilter.org>
>> Cc: Patrick McHardy <kaber@trash.net>
>> Cc: "David S. Miller" <davem@davemloft.net>
>> ---
>>  net/netfilter/ipvs/ip_vs_ftp.c |    2 ++
>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
>> index b20b29c..c2bc264 100644
>> --- a/net/netfilter/ipvs/ip_vs_ftp.c
>> +++ b/net/netfilter/ipvs/ip_vs_ftp.c
>> @@ -65,8 +65,10 @@ static int ip_vs_ftp_pasv;
>>  static int
>>  ip_vs_ftp_init_conn(struct ip_vs_app *app, struct ip_vs_conn *cp)
>>  {
>> +     spin_lock(&cp->lock);
>>       /* We use connection tracking for the command connection */
>>       cp->flags |= IP_VS_CONN_F_NFCT;
>> +     spin_unlock(&cp->lock);
>>       return 0;
>>  }
>>
>> --
>> 1.7.1
>
> Regards
>
> --
> Julian Anastasov <ja@ssi.bg>

^ permalink raw reply

* [PATCH] ipv4: Adjust in_dev handling in fib_validate_source()
From: David Miller @ 2012-06-29  2:05 UTC (permalink / raw)
  To: netdev


Checking for in_dev being NULL is pointless.

In fact, all of our callers have in_dev precomputed already,
so just pass it in and remove the NULL checking.

Signed-off-by: David S. Miller <davem@davemloft.net>
---
 include/net/ip_fib.h    |    2 +-
 net/ipv4/fib_frontend.c |   27 ++++++++++-----------------
 net/ipv4/route.c        |   10 ++++++----
 3 files changed, 17 insertions(+), 22 deletions(-)

diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index 9e6c26d..619f68a 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -233,7 +233,7 @@ extern void		ip_fib_init(void);
 extern __be32 fib_compute_spec_dst(struct sk_buff *skb);
 extern int fib_validate_source(struct sk_buff *skb, __be32 src, __be32 dst,
 			       u8 tos, int oif, struct net_device *dev,
-			       u32 *itag);
+			       struct in_device *idev, u32 *itag);
 extern void fib_select_default(struct fib_result *res);
 
 /* Exported by fib_semantics.c */
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index 1d13217..c84cff5 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -226,15 +226,14 @@ __be32 fib_compute_spec_dst(struct sk_buff *skb)
  * called with rcu_read_lock()
  */
 int fib_validate_source(struct sk_buff *skb, __be32 src, __be32 dst, u8 tos,
-			int oif, struct net_device *dev, u32 *itag)
+			int oif, struct net_device *dev, struct in_device *idev,
+			u32 *itag)
 {
-	struct in_device *in_dev;
-	struct flowi4 fl4;
+	int ret, no_addr, rpf, accept_local;
 	struct fib_result res;
-	int no_addr, rpf, accept_local;
-	bool dev_match;
-	int ret;
+	struct flowi4 fl4;
 	struct net *net;
+	bool dev_match;
 
 	fl4.flowi4_oif = 0;
 	fl4.flowi4_iif = oif;
@@ -244,19 +243,13 @@ int fib_validate_source(struct sk_buff *skb, __be32 src, __be32 dst, u8 tos,
 	fl4.flowi4_scope = RT_SCOPE_UNIVERSE;
 
 	no_addr = rpf = accept_local = 0;
-	in_dev = __in_dev_get_rcu(dev);
-	if (in_dev) {
-		no_addr = in_dev->ifa_list == NULL;
-
-		/* Ignore rp_filter for packets protected by IPsec. */
-		rpf = secpath_exists(skb) ? 0 : IN_DEV_RPFILTER(in_dev);
+	no_addr = idev->ifa_list == NULL;
 
-		accept_local = IN_DEV_ACCEPT_LOCAL(in_dev);
-		fl4.flowi4_mark = IN_DEV_SRC_VMARK(in_dev) ? skb->mark : 0;
-	}
+	/* Ignore rp_filter for packets protected by IPsec. */
+	rpf = secpath_exists(skb) ? 0 : IN_DEV_RPFILTER(idev);
 
-	if (in_dev == NULL)
-		goto e_inval;
+	accept_local = IN_DEV_ACCEPT_LOCAL(idev);
+	fl4.flowi4_mark = IN_DEV_SRC_VMARK(idev) ? skb->mark : 0;
 
 	net = dev_net(dev);
 	if (fib_lookup(net, &fl4, &res))
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 83d56a0..919d69e 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1999,7 +1999,8 @@ static int ip_route_input_mc(struct sk_buff *skb, __be32 daddr, __be32 saddr,
 		if (!ipv4_is_local_multicast(daddr))
 			goto e_inval;
 	} else {
-		err = fib_validate_source(skb, saddr, 0, tos, 0, dev, &itag);
+		err = fib_validate_source(skb, saddr, 0, tos, 0, dev,
+					  in_dev, &itag);
 		if (err < 0)
 			goto e_err;
 	}
@@ -2100,7 +2101,7 @@ static int __mkroute_input(struct sk_buff *skb,
 
 
 	err = fib_validate_source(skb, saddr, daddr, tos, FIB_RES_OIF(*res),
-				  in_dev->dev, &itag);
+				  in_dev->dev, in_dev, &itag);
 	if (err < 0) {
 		ip_handle_martian_source(in_dev->dev, in_dev, skb, daddr,
 					 saddr);
@@ -2274,7 +2275,7 @@ static int ip_route_input_slow(struct sk_buff *skb, __be32 daddr, __be32 saddr,
 	if (res.type == RTN_LOCAL) {
 		err = fib_validate_source(skb, saddr, daddr, tos,
 					  net->loopback_dev->ifindex,
-					  dev, &itag);
+					  dev, in_dev, &itag);
 		if (err < 0)
 			goto martian_source_keep_err;
 		if (err)
@@ -2295,7 +2296,8 @@ brd_input:
 		goto e_inval;
 
 	if (!ipv4_is_zeronet(saddr)) {
-		err = fib_validate_source(skb, saddr, 0, tos, 0, dev, &itag);
+		err = fib_validate_source(skb, saddr, 0, tos, 0, dev,
+					  in_dev, &itag);
 		if (err < 0)
 			goto martian_source_keep_err;
 		if (err)
-- 
1.7.10

^ permalink raw reply related

* "ADDRCONF(NETDEV_UP): eth0: link is not ready" with IPv6
From: Arvid Brodin @ 2012-06-29  2:36 UTC (permalink / raw)
  To: netdev@vger.kernel.org; +Cc: Alexey Kuznetsov, Stephen Hemminger

Hi,

After 'ip link set eth0 up' on an avr32 board (network driver macb), the device ends up in
operational mode "UNKNOWN":

# ip link
2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UNKNOWN qlen 1000
    link/ether 00:24:74:00:17:9d brd ff:ff:ff:ff:ff:ff

Unplugging and plugging in the network cable gets the device to mode "UP".

This is a problem for me because I'm trying to use this device as a "slave" device (for a
virtual HSR device*) and I need to be able to decide if the slave device is operational or
not.

Following Stephen's advice here:
http://kerneltrap.org/mailarchive/linux-netdev/2008/9/24/3398834 I checked the macb.c code
and noticed they do not call netif_carrier_off() neither before register_netdev() nor in
dev_open().

I added the call before register_netdev(), which fixed the problem. However, if I then
enable IPv6:

# ip link set eth0 up
ADDRCONF(NETDEV_UP): eth0: link is not ready
eth0: link up (100/Full)
ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready

Any idea what is happening / what I'm doing wrong? (This is not just cosmetic; is some
situations this seems to kill the interface - e.g. ping does not work, down/up does not
help...) Things work fine without IPv6 configured.


*N.B. I'm writing a driver for a network protocol called "High-availability Seamless
Redundancy".


-- 
Arvid Brodin | Consultant (Linux)
XDIN AB | Jan Stenbecks Torg 17 | SE-164 40 Kista | Sweden | xdin.com

^ permalink raw reply

* [PATCH] ipv6_tunnel: Allow receiving packets on the fallback tunnel if they pass sanity checks
From: Phil Dibowitz @ 2012-06-29  4:15 UTC (permalink / raw)
  To: davem; +Cc: netdev, phild, ville.nuorvala, phil

From: Ville Nuorvala <ville.nuorvala@gmail.com>

At Facebook, we do Layer-3 DSR via IP-in-IP tunneling. Our load balancers wrap
an extra IP header on incoming packets so they can be routed to the backend.
In the v4 tunnel driver, when these packets fall on the default tunl0 device,
the behavior is to decapsulate them and drop them back on the stack. So our
setup is that tunl0 has the VIP and eth0 has (obviously) the backend's real
address.

In IPv6 we do the same thing, but the v6 tunnel driver didn't have this same
behavior - if you didn't have an explicit tunnel setup, it would drop the
packet.

This patch brings that v4 feature to the v6 driver.

The same IPv6 address checks are performed as with any normal tunnel,
but as the fallback tunnel endpoint addresses are unspecified, the checks
must be performed on a per-packet basis, rather than at tunnel
configuration time.

[Patch description modified by phil@ipom.com]

Signed-off-by: Ville Nuorvala <ville.nuorvala@gmail.com>
Tested-by: Phil Dibowitz <phil@ipom.com>

---
 include/net/ip6_tunnel.h |    2 +
 net/ipv6/ip6_tunnel.c    |   65 +++++++++++++++++++++++++++-------------------
 2 files changed, 40 insertions(+), 27 deletions(-)

diff --git a/include/net/ip6_tunnel.h b/include/net/ip6_tunnel.h
index fc73e66..358fb86 100644
--- a/include/net/ip6_tunnel.h
+++ b/include/net/ip6_tunnel.h
@@ -9,6 +9,8 @@
 #define IP6_TNL_F_CAP_XMIT 0x10000
 /* capable of receiving packets */
 #define IP6_TNL_F_CAP_RCV 0x20000
+/* determine capability on a per-packet basis */
+#define IP6_TNL_F_CAP_PER_PACKET 0x40000
 
 /* IPv6 tunnel */
 
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index c9015fa..04a3cba 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -684,24 +684,50 @@ static void ip6ip6_dscp_ecn_decapsulate(const struct ip6_tnl *t,
 		IP6_ECN_set_ce(ipv6_hdr(skb));
 }
 
+static __u32 ip6_tnl_get_cap(struct ip6_tnl *t,
+			     const struct in6_addr *laddr,
+			     const struct in6_addr *raddr)
+{
+	struct ip6_tnl_parm *p = &t->parms;
+	int ltype = ipv6_addr_type(laddr);
+	int rtype = ipv6_addr_type(raddr);
+	__u32 flags = 0;
+
+	if (ltype == IPV6_ADDR_ANY || rtype == IPV6_ADDR_ANY) {
+		flags = IP6_TNL_F_CAP_PER_PACKET;
+	} else if (ltype & (IPV6_ADDR_UNICAST|IPV6_ADDR_MULTICAST) &&
+		   rtype & (IPV6_ADDR_UNICAST|IPV6_ADDR_MULTICAST) &&
+		   !((ltype|rtype) & IPV6_ADDR_LOOPBACK) &&
+		   (!((ltype|rtype) & IPV6_ADDR_LINKLOCAL) || p->link)) {
+		if (ltype&IPV6_ADDR_UNICAST)
+			flags |= IP6_TNL_F_CAP_XMIT;
+		if (rtype&IPV6_ADDR_UNICAST)
+			flags |= IP6_TNL_F_CAP_RCV;
+	}
+	return flags;
+}
+
 /* called with rcu_read_lock() */
-static inline int ip6_tnl_rcv_ctl(struct ip6_tnl *t)
+static inline int ip6_tnl_rcv_ctl(struct ip6_tnl *t,
+				  const struct in6_addr *laddr,
+				  const struct in6_addr *raddr)
 {
 	struct ip6_tnl_parm *p = &t->parms;
 	int ret = 0;
 	struct net *net = dev_net(t->dev);
 
-	if (p->flags & IP6_TNL_F_CAP_RCV) {
+	if ((p->flags & IP6_TNL_F_CAP_RCV) ||
+	    ((p->flags & IP6_TNL_F_CAP_PER_PACKET) &&
+	     (ip6_tnl_get_cap(t, laddr, raddr) & IP6_TNL_F_CAP_RCV))) {
 		struct net_device *ldev = NULL;
 
 		if (p->link)
 			ldev = dev_get_by_index_rcu(net, p->link);
 
-		if ((ipv6_addr_is_multicast(&p->laddr) ||
-		     likely(ipv6_chk_addr(net, &p->laddr, ldev, 0))) &&
-		    likely(!ipv6_chk_addr(net, &p->raddr, NULL, 0)))
+		if ((ipv6_addr_is_multicast(laddr) ||
+		     likely(ipv6_chk_addr(net, laddr, ldev, 0))) &&
+		    likely(!ipv6_chk_addr(net, raddr, NULL, 0)))
 			ret = 1;
-
 	}
 	return ret;
 }
@@ -740,7 +766,7 @@ static int ip6_tnl_rcv(struct sk_buff *skb, __u16 protocol,
 			goto discard;
 		}
 
-		if (!ip6_tnl_rcv_ctl(t)) {
+		if (!ip6_tnl_rcv_ctl(t, &ipv6h->daddr, &ipv6h->saddr)) {
 			t->dev->stats.rx_dropped++;
 			rcu_read_unlock();
 			goto discard;
@@ -1114,25 +1140,6 @@ tx_err:
 	return NETDEV_TX_OK;
 }
 
-static void ip6_tnl_set_cap(struct ip6_tnl *t)
-{
-	struct ip6_tnl_parm *p = &t->parms;
-	int ltype = ipv6_addr_type(&p->laddr);
-	int rtype = ipv6_addr_type(&p->raddr);
-
-	p->flags &= ~(IP6_TNL_F_CAP_XMIT|IP6_TNL_F_CAP_RCV);
-
-	if (ltype & (IPV6_ADDR_UNICAST|IPV6_ADDR_MULTICAST) &&
-	    rtype & (IPV6_ADDR_UNICAST|IPV6_ADDR_MULTICAST) &&
-	    !((ltype|rtype) & IPV6_ADDR_LOOPBACK) &&
-	    (!((ltype|rtype) & IPV6_ADDR_LINKLOCAL) || p->link)) {
-		if (ltype&IPV6_ADDR_UNICAST)
-			p->flags |= IP6_TNL_F_CAP_XMIT;
-		if (rtype&IPV6_ADDR_UNICAST)
-			p->flags |= IP6_TNL_F_CAP_RCV;
-	}
-}
-
 static void ip6_tnl_link_config(struct ip6_tnl *t)
 {
 	struct net_device *dev = t->dev;
@@ -1153,7 +1160,8 @@ static void ip6_tnl_link_config(struct ip6_tnl *t)
 	if (!(p->flags&IP6_TNL_F_USE_ORIG_FLOWLABEL))
 		fl6->flowlabel |= IPV6_FLOWLABEL_MASK & p->flowinfo;
 
-	ip6_tnl_set_cap(t);
+	p->flags &= ~(IP6_TNL_F_CAP_XMIT|IP6_TNL_F_CAP_RCV|IP6_TNL_F_CAP_PER_PACKET);
+	p->flags |= ip6_tnl_get_cap(t, &p->laddr, &p->raddr);
 
 	if (p->flags&IP6_TNL_F_CAP_XMIT && p->flags&IP6_TNL_F_CAP_RCV)
 		dev->flags |= IFF_POINTOPOINT;
@@ -1438,6 +1446,9 @@ static int __net_init ip6_fb_tnl_dev_init(struct net_device *dev)
 
 	t->parms.proto = IPPROTO_IPV6;
 	dev_hold(dev);
+
+	ip6_tnl_link_config(t);
+
 	rcu_assign_pointer(ip6n->tnls_wc[0], t);
 	return 0;
 }

^ permalink raw reply related

* Re: [PATCH net-next] fq_codel: report congestion notification at enqueue time
From: Eric Dumazet @ 2012-06-29  4:18 UTC (permalink / raw)
  To: Nandita Dukkipati
  Cc: netdev, Yuchung Cheng, codel, Matt Mathis, Neal Cardwell,
	David Miller
In-Reply-To: <CAB_+Fg7x2pV72Wa45QLuokHTVPuzdmrMfwiuLjS_vi02vtasAA@mail.gmail.com>

On Thu, 2012-06-28 at 16:52 -0700, Nandita Dukkipati wrote:
> As you know I really like this idea. My main concern is that the same
> packet could cause TCP to reduce cwnd twice within an RTT - first on
> enqueue and then if this packet is ECN marked on dequeue. I don't
> think this is the desired behavior. Can we avoid it?

I'll work on this.

In my experiences, I found that no drops (or ECN marks) were done at
dequeue time once one NET_XMIT_CN was returned, but its certainly
possible if other flows compete with this one.

Strangely, SFQ has the same behavior and nobody complained yet ;)

Thanks

^ permalink raw reply

* Re: [PATCH net-next] fq_codel: report congestion notification at enqueue time
From: Eric Dumazet @ 2012-06-29  4:50 UTC (permalink / raw)
  To: Dave Taht
  Cc: Nandita Dukkipati, netdev, Yuchung Cheng, codel, Matt Mathis,
	Neal Cardwell, David Miller
In-Reply-To: <CAA93jw7ywd0auqN=k2J4fLOGYMLRH+agWr4P+_CisgvRtvgJUw@mail.gmail.com>

On Thu, 2012-06-28 at 19:47 -0400, Dave Taht wrote: 
> On Thu, Jun 28, 2012 at 6:56 PM, Yuchung Cheng <ycheng@google.com> wrote:
> > On Thu, Jun 28, 2012 at 11:12 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> >> On Thu, 2012-06-28 at 10:51 -0700, Dave Taht wrote:
> >>
> >>> clever idea. A problem is there are other forms of network traffic on
> >>> a link, and this is punishing a single tcp
> > Dave: it won't just punish a single TCP, all protocols that react to
> > XMIT_CN will share similar fate.
> 
> What protocols in the kernel do and don't? was the crux of this question.
> 

AFAIK that only tcp cares a bit, or seems to.

But not that much, since it continues to send packets.

Thats because tcp_transmit_skb() changes the NET_XMIT_CN status to plain
NET_XMIT_SUCCESS.

My long term plan is to reduce number of skbs queued in Qdisc for TCP
stack, to reduce RTT (removing the artificial RTT bias because of local
queues)

> I'm not objecting to the idea, it's clever, as I said. I'm thinking I'll
> apply it to cerowrt's next build and see what happens, if this
> will apply against 3.3. Or maybe the ns3 model. Or both.

A router will have no use of this feature, not sure you need to spend
time trying this ;)

^ permalink raw reply

* Re: [PATCH net-next] fq_codel: report congestion notification at enqueue time
From: Eric Dumazet @ 2012-06-29  4:53 UTC (permalink / raw)
  To: David Miller
  Cc: Nandita Dukkipati, netdev, codel, Yuchung Cheng, Neal Cardwell,
	Matt Mathis
In-Reply-To: <1340903237.13187.151.camel@edumazet-glaptop>

On Thu, 2012-06-28 at 19:07 +0200, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
> 
> At enqueue time, check sojourn time of packet at head of the queue,
> and return NET_XMIT_CN instead of NET_XMIT_SUCCESS if this sejourn
> time is above codel @target.
> 
> This permits local TCP stack to call tcp_enter_cwr() and reduce its cwnd
> without drops (for example if ECN is not enabled for the flow)
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Dave Taht <dave.taht@bufferbloat.net>
> Cc: Tom Herbert <therbert@google.com>
> Cc: Matt Mathis <mattmathis@google.com>
> Cc: Yuchung Cheng <ycheng@google.com>
> Cc: Nandita Dukkipati <nanditad@google.com>
> Cc: Neal Cardwell <ncardwell@google.com>
> ---

Please dont apply this patch, I'll submit an updated version later.

Thanks

^ permalink raw reply

* Re: [PATCH net-next] fq_codel: report congestion notification at enqueue time
From: David Miller @ 2012-06-29  5:12 UTC (permalink / raw)
  To: eric.dumazet; +Cc: nanditad, netdev, codel, ycheng, ncardwell, mattmathis
In-Reply-To: <1340945592.29822.8.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 29 Jun 2012 06:53:12 +0200

> Please dont apply this patch, I'll submit an updated version later.

Ok.

^ permalink raw reply

* Re: [PATCH net-next] fq_codel: report congestion notification at enqueue time
From: Eric Dumazet @ 2012-06-29  5:24 UTC (permalink / raw)
  To: David Miller; +Cc: nanditad, netdev, codel, ycheng, ncardwell, mattmathis
In-Reply-To: <20120628.221252.2220466000873887315.davem@davemloft.net>

On Thu, 2012-06-28 at 22:12 -0700, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Fri, 29 Jun 2012 06:53:12 +0200
> 
> > Please dont apply this patch, I'll submit an updated version later.
> 
> Ok.

By the way, I am not sure NET_XMIT_CN is correctly used in RED.

Or maybe my understanding of NET_XMIT_CN is wrong.

If a the packet is dropped in enqueue(), why use NET_XMIT_CN instead of 
NET_XMIT_DROP ?

This seems to mean : I dropped _this_ packet, but dont worry too much, I
might accept other packets, so please go on...

diff --git a/net/sched/sch_red.c b/net/sched/sch_red.c
index 633e32d..0fc5b6c 100644
--- a/net/sched/sch_red.c
+++ b/net/sched/sch_red.c
@@ -77,7 +77,7 @@ static int red_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 		sch->qstats.overlimits++;
 		if (!red_use_ecn(q) || !INET_ECN_set_ce(skb)) {
 			q->stats.prob_drop++;
-			goto congestion_drop;
+			goto drop;
 		}
 
 		q->stats.prob_mark++;
@@ -88,7 +88,7 @@ static int red_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 		if (red_use_harddrop(q) || !red_use_ecn(q) ||
 		    !INET_ECN_set_ce(skb)) {
 			q->stats.forced_drop++;
-			goto congestion_drop;
+			goto drop;
 		}
 
 		q->stats.forced_mark++;
@@ -104,9 +104,8 @@ static int red_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 	}
 	return ret;
 
-congestion_drop:
-	qdisc_drop(skb, sch);
-	return NET_XMIT_CN;
+drop:
+	return qdisc_drop(skb, sch);
 }
 
 static struct sk_buff *red_dequeue(struct Qdisc *sch)

^ permalink raw reply related

* [PATCH] ipv4: Remove extraneous assignment of dst->tclassid.
From: David Miller @ 2012-06-29  5:24 UTC (permalink / raw)
  To: netdev


We already set it several lines above.

Signed-off-by: David S. Miller <davem@davemloft.net>
---
 net/ipv4/route.c |    3 ---
 1 file changed, 3 deletions(-)

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 919d69e..6a5afc7 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2327,9 +2327,6 @@ local_input:
 	rth->rt_key_tos	= tos;
 	rth->rt_dst	= daddr;
 	rth->rt_src	= saddr;
-#ifdef CONFIG_IP_ROUTE_CLASSID
-	rth->dst.tclassid = itag;
-#endif
 	rth->rt_route_iif = dev->ifindex;
 	rth->rt_iif	= dev->ifindex;
 	rth->rt_oif	= 0;
-- 
1.7.10

^ permalink raw reply related

* Re: [PATCH net-next] fq_codel: report congestion notification at enqueue time
From: Dave Taht @ 2012-06-29  5:24 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Nandita Dukkipati, netdev, Yuchung Cheng, codel, Matt Mathis,
	Neal Cardwell, David Miller
In-Reply-To: <1340945457.29822.7.camel@edumazet-glaptop>

On Fri, Jun 29, 2012 at 12:50 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:

> A router will have no use of this feature, not sure you need to spend
> time trying this ;)

It's not yer ordinary router...

A cerowrt router has iperf, netperf/netserver from svn with congestion
control switching and classification setting, rsync (with same),
samba, transmission, a polipo proxy, scamper, and a legion of other
network analysis tools on-board and available as optional packages.

and it's used in the bufferbloat project as a thoroughly understood
platform for originating, receiving, AND routing packets on a real
embedded home gateway platform that end users actually use, through a
decent set of drivers, on ethernet and wifi.

I am always concerned when changes to the stack like
GSO/GRO/BQL/fq_codel go into linux - or things like the infinite
window in ECN bug from a few months back happen - as they hold promise
to mutate (or explain) the statistics and analysis we've accumulated
over the last year and a half.

And as I'm hoping to do a major test run shortly to get some fresh
statistics vs a vs fq_codel vs the old sfqred tests ( I'm looking
forward to redoing this one in particular:
http://www.teklibre.com/~d/bloat/hoqvssfqred.ps - )

... and you are about to change what those stats are going to look
like, under load, with this change... I kind of need to
understand/track it/parse it/capture it. I've got sufficient hardware
now to easily A/B things.

(sorry for the noise on the lists)


-- 
Dave Täht

^ permalink raw reply

* Re: [PATCH net-next] fq_codel: report congestion notification at enqueue time
From: David Miller @ 2012-06-29  5:29 UTC (permalink / raw)
  To: eric.dumazet; +Cc: nanditad, netdev, codel, ycheng, ncardwell, mattmathis
In-Reply-To: <1340947448.29822.41.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 29 Jun 2012 07:24:08 +0200

> By the way, I am not sure NET_XMIT_CN is correctly used in RED.
> 
> Or maybe my understanding of NET_XMIT_CN is wrong.
> 
> If a the packet is dropped in enqueue(), why use NET_XMIT_CN instead of 
> NET_XMIT_DROP ?
> 
> This seems to mean : I dropped _this_ packet, but dont worry too much, I
> might accept other packets, so please go on...

I am pretty sure the behavior in RED is intentional.

It's a soft push back on TCP.

We're taking this path when we are unable to sucessfully ECN mark a
packet.  But our intention was to do so.

^ permalink raw reply

* Re: [PATCH net-next] fq_codel: report congestion notification at enqueue time
From: Eric Dumazet @ 2012-06-29  5:50 UTC (permalink / raw)
  To: David Miller; +Cc: nanditad, netdev, codel, ycheng, ncardwell, mattmathis
In-Reply-To: <20120628.222934.767995619021650710.davem@davemloft.net>

On Thu, 2012-06-28 at 22:29 -0700, David Miller wrote:

> I am pretty sure the behavior in RED is intentional.
> 
> It's a soft push back on TCP.
> 

tcp_enter_cwr() is called the same for DROP and CN

> We're taking this path when we are unable to sucessfully ECN mark a
> packet.  But our intention was to do so.
> 

Hmm, problem is the sender thinks the packet was queued for
transmission.

        ret = macvlan_queue_xmit(skb, dev);
        if (likely(ret == NET_XMIT_SUCCESS || ret == NET_XMIT_CN)) {
                struct macvlan_pcpu_stats *pcpu_stats;

                pcpu_stats = this_cpu_ptr(vlan->pcpu_stats);
                u64_stats_update_begin(&pcpu_stats->syncp);
                pcpu_stats->tx_packets++;
                pcpu_stats->tx_bytes += len;
                u64_stats_update_end(&pcpu_stats->syncp);
        } else {
                this_cpu_inc(vlan->pcpu_stats->tx_dropped);
        }

NET_XMIT_CN has a lazy semantic it seems.

I will just dont rely on it.

^ permalink raw reply

* [PATCH net-next] net: l2tp_eth: provide tx_dropped counter
From: Eric Dumazet @ 2012-06-29  6:15 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

From: Eric Dumazet <edumazet@google.com>

Change l2tp_xmit_skb() to return NET_XMIT_DROP in case skb is dropped.

Use kfree_skb() instead dev_kfree_skb() for drop_monitor pleasure.

Support tx_dropped counter for l2tp_eth

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: James Chapman <jchapman@katalix.com>
---
 net/l2tp/l2tp_core.c |   11 ++++++-----
 net/l2tp/l2tp_eth.c  |   15 ++++++++++-----
 2 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 32b2155..393355d 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -1128,6 +1128,7 @@ int l2tp_xmit_skb(struct l2tp_session *session, struct sk_buff *skb, int hdr_len
 	int headroom;
 	int uhlen = (tunnel->encap == L2TP_ENCAPTYPE_UDP) ? sizeof(struct udphdr) : 0;
 	int udp_len;
+	int ret = NET_XMIT_SUCCESS;
 
 	/* Check that there's enough headroom in the skb to insert IP,
 	 * UDP and L2TP headers. If not enough, expand it to
@@ -1137,8 +1138,8 @@ int l2tp_xmit_skb(struct l2tp_session *session, struct sk_buff *skb, int hdr_len
 		uhlen + hdr_len;
 	old_headroom = skb_headroom(skb);
 	if (skb_cow_head(skb, headroom)) {
-		dev_kfree_skb(skb);
-		goto abort;
+		kfree_skb(skb);
+		return NET_XMIT_DROP;
 	}
 
 	new_headroom = skb_headroom(skb);
@@ -1156,7 +1157,8 @@ int l2tp_xmit_skb(struct l2tp_session *session, struct sk_buff *skb, int hdr_len
 
 	bh_lock_sock(sk);
 	if (sock_owned_by_user(sk)) {
-		dev_kfree_skb(skb);
+		kfree_skb(skb);
+		ret = NET_XMIT_DROP;
 		goto out_unlock;
 	}
 
@@ -1215,8 +1217,7 @@ int l2tp_xmit_skb(struct l2tp_session *session, struct sk_buff *skb, int hdr_len
 out_unlock:
 	bh_unlock_sock(sk);
 
-abort:
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL_GPL(l2tp_xmit_skb);
 
diff --git a/net/l2tp/l2tp_eth.c b/net/l2tp/l2tp_eth.c
index 47b259f..f9ee74d 100644
--- a/net/l2tp/l2tp_eth.c
+++ b/net/l2tp/l2tp_eth.c
@@ -44,6 +44,7 @@ struct l2tp_eth {
 	struct list_head	list;
 	atomic_long_t		tx_bytes;
 	atomic_long_t		tx_packets;
+	atomic_long_t		tx_dropped;
 	atomic_long_t		rx_bytes;
 	atomic_long_t		rx_packets;
 	atomic_long_t		rx_errors;
@@ -92,12 +93,15 @@ static int l2tp_eth_dev_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct l2tp_eth *priv = netdev_priv(dev);
 	struct l2tp_session *session = priv->session;
+	unsigned int len = skb->len;
+	int ret = l2tp_xmit_skb(session, skb, session->hdr_len);
 
-	atomic_long_add(skb->len, &priv->tx_bytes);
-	atomic_long_inc(&priv->tx_packets);
-
-	l2tp_xmit_skb(session, skb, session->hdr_len);
-
+	if (likely(ret == NET_XMIT_SUCCESS)) {
+		atomic_long_add(len, &priv->tx_bytes);
+		atomic_long_inc(&priv->tx_packets);
+	} else {
+		atomic_long_inc(&priv->tx_dropped);
+	}
 	return NETDEV_TX_OK;
 }
 
@@ -108,6 +112,7 @@ static struct rtnl_link_stats64 *l2tp_eth_get_stats64(struct net_device *dev,
 
 	stats->tx_bytes   = atomic_long_read(&priv->tx_bytes);
 	stats->tx_packets = atomic_long_read(&priv->tx_packets);
+	stats->tx_dropped = atomic_long_read(&priv->tx_dropped);
 	stats->rx_bytes   = atomic_long_read(&priv->rx_bytes);
 	stats->rx_packets = atomic_long_read(&priv->rx_packets);
 	stats->rx_errors  = atomic_long_read(&priv->rx_errors);

^ permalink raw reply related

* [PATCH net-next 1/1] netxen_nic: restrict force firmware dump when dump is disabled.
From: Rajesh Borundia @ 2012-06-29  6:12 UTC (permalink / raw)
  To: davem; +Cc: netdev, ameen.rahman, Manish chopra
In-Reply-To: <1340950341-27252-1-git-send-email-rajesh.borundia@qlogic.com>

From: Manish chopra <manish.chopra@qlogic.com>

o Set the ethtool_dump flag (=ETH_FW_DUMP_DISABLE) when dump is disabled.
o update driver version to 4.0.80

Signed-off-by: Manish chopra <manish.chopra@qlogic.com>
Signed-off-by: Rajesh Borundia <rajesh.borundia@qlogic.com>
---
 drivers/net/ethernet/qlogic/netxen/netxen_nic.h    |    4 ++--
 .../ethernet/qlogic/netxen/netxen_nic_ethtool.c    |   13 ++++++++++---
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic.h b/drivers/net/ethernet/qlogic/netxen/netxen_nic.h
index 37ccbe5..eb3dfdb 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic.h
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic.h
@@ -53,8 +53,8 @@
 
 #define _NETXEN_NIC_LINUX_MAJOR 4
 #define _NETXEN_NIC_LINUX_MINOR 0
-#define _NETXEN_NIC_LINUX_SUBVERSION 79
-#define NETXEN_NIC_LINUX_VERSIONID  "4.0.79"
+#define _NETXEN_NIC_LINUX_SUBVERSION 80
+#define NETXEN_NIC_LINUX_VERSIONID  "4.0.80"
 
 #define NETXEN_VERSION_CODE(a, b, c)	(((a) << 24) + ((b) << 16) + (c))
 #define _major(v)	(((v) >> 24) & 0xff)
diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_ethtool.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_ethtool.c
index 9103e3e..10468e7 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_ethtool.c
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_ethtool.c
@@ -826,7 +826,12 @@ netxen_get_dump_flag(struct net_device *netdev, struct ethtool_dump *dump)
 		dump->len = mdump->md_dump_size;
 	else
 		dump->len = 0;
-	dump->flag = mdump->md_capture_mask;
+
+	if (!mdump->md_enabled)
+		dump->flag = ETH_FW_DUMP_DISABLE;
+	else
+		dump->flag = mdump->md_capture_mask;
+
 	dump->version = adapter->fw_version;
 	return 0;
 }
@@ -840,8 +845,10 @@ netxen_set_dump(struct net_device *netdev, struct ethtool_dump *val)
 
 	switch (val->flag) {
 	case NX_FORCE_FW_DUMP_KEY:
-		if (!mdump->md_enabled)
-			mdump->md_enabled = 1;
+		if (!mdump->md_enabled) {
+			netdev_info(netdev, "FW dump not enabled\n");
+			return 0;
+		}
 		if (adapter->fw_mdump_rdy) {
 			netdev_info(netdev, "Previous dump not cleared, not forcing dump\n");
 			return 0;
-- 
1.7.3.3

^ permalink raw reply related

* [PATCH net-next 0/1] netxen: bug fix
From: Rajesh Borundia @ 2012-06-29  6:12 UTC (permalink / raw)
  To: davem; +Cc: netdev, ameen.rahman

Please apply it to net-next.

Thanks,
Rajesh

^ permalink raw reply

* [PATCH net-next] caif-hsi: Fix merge issues.
From: sjur.brandeland @ 2012-06-29  6:36 UTC (permalink / raw)
  To: davem; +Cc: netdev, sjurbren, Sjur Brændeland

From: Sjur Brændeland <sjur.brandeland@stericsson.com>

Fix the failing merge in net-next by reverting the last
net-next merge for caif_hsi.c and then merge in the commit:
"caif-hsi: Bugfix - Piggyback'ed embedded CAIF frame lost"
from the net repository. 

The commit:"caif-hsi: Add missing return in error path" from
net repository was dropped, as it changed code previously removed in the 
net-next repository.

Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>

---
Hi Dave,

>Sjur please send me any necessary fixups and please be more mindful in
>the future of the incredible merge pain you put me through when you
>have such fundamentally overlapping changes like that and don't
>provide me with a sample merge resolution like other people do.

Yes this merge went really bad, sorry for the pain I have caused you!
I'll try to provide you with merge instruction or sample merge
resolutions in the future if there are conflicts.

Regards,
Sjur


---
 drivers/net/caif/caif_hsi.c |   72 ++++++++++++++++---------------------------
 1 files changed, 27 insertions(+), 45 deletions(-)

diff --git a/drivers/net/caif/caif_hsi.c b/drivers/net/caif/caif_hsi.c
index 087eb83..0def8b3 100644
--- a/drivers/net/caif/caif_hsi.c
+++ b/drivers/net/caif/caif_hsi.c
@@ -1131,51 +1131,7 @@ static void cfhsi_setup(struct net_device *dev)
 	cfhsi->cfdev.use_stx = false;
 	cfhsi->cfdev.use_fcs = false;
 	cfhsi->ndev = dev;
-}
-
-int cfhsi_probe(struct platform_device *pdev)
-{
-	struct cfhsi_ops *(*get_ops)(void);
-	struct cfhsi *cfhsi = NULL;
-	struct net_device *ndev;
-	int res;
-
-	ndev = alloc_netdev(sizeof(struct cfhsi), "cfhsi%d", cfhsi_setup);
-	if (!ndev)
-		return -ENODEV;
-
-	cfhsi = netdev_priv(ndev);
-	cfhsi->ndev = ndev;
-	cfhsi->pdev = pdev;
-
-	get_ops = symbol_get(cfhsi_get_ops);
-	if (!get_ops) {
-		pr_err("%s: failed to get the cfhsi_ops\n", __func__);
-		return -ENODEV;
-	}
-
-	/* Assign the HSI device. */
-	cfhsi->ops = (*get_ops)();
-	if (!cfhsi->ops) {
-		pr_err("%s: failed to get the cfhsi_ops\n", __func__);
-		goto err;
-	}
-
-	/* Assign the driver to this HSI device. */
-	cfhsi->ops->cb_ops = &cfhsi->cb_ops;
-	res = register_netdevice(ndev);
-	if (res) {
-		dev_err(&ndev->dev, "%s: Registration error: %d.\n",
-			__func__, res);
-		free_netdev(ndev);
-	}
-	/* Add CAIF HSI device to list. */
-	list_add_tail(&cfhsi->list, &cfhsi_list);
-
-	return res;
-err:
-	symbol_put(cfhsi_get_ops);
-	return -ENODEV;
+	cfhsi->cfg = hsi_default_config;
 }
 
 static int cfhsi_open(struct net_device *ndev)
@@ -1454,6 +1410,7 @@ static int caif_hsi_newlink(struct net *src_net, struct net_device *dev,
 			  struct nlattr *tb[], struct nlattr *data[])
 {
 	struct cfhsi *cfhsi = NULL;
+	struct cfhsi_ops *(*get_ops)(void);
 
 	ASSERT_RTNL();
 
@@ -1461,7 +1418,32 @@ static int caif_hsi_newlink(struct net *src_net, struct net_device *dev,
 	cfhsi_netlink_parms(data, cfhsi);
 	dev_net_set(cfhsi->ndev, src_net);
 
+	get_ops = symbol_get(cfhsi_get_ops);
+	if (!get_ops) {
+		pr_err("%s: failed to get the cfhsi_ops\n", __func__);
+		return -ENODEV;
+	}
+
+	/* Assign the HSI device. */
+	cfhsi->ops = (*get_ops)();
+	if (!cfhsi->ops) {
+		pr_err("%s: failed to get the cfhsi_ops\n", __func__);
+		goto err;
+	}
+
+	/* Assign the driver to this HSI device. */
+	cfhsi->ops->cb_ops = &cfhsi->cb_ops;
+	if (register_netdevice(dev)) {
+		pr_warn("%s: caif_hsi device registration failed\n", __func__);
+		goto err;
+	}
+	/* Add CAIF HSI device to list. */
+	list_add_tail(&cfhsi->list, &cfhsi_list);
+
 	return 0;
+err:
+	symbol_put(cfhsi_get_ops);
+	return -ENODEV;
 }
 
 static struct rtnl_link_ops caif_hsi_link_ops __read_mostly = {
-- 
1.7.5.4

^ permalink raw reply related

* Re: [patch net-next 1/4] net: introduce new priv_flag indicating iface capable of change mac when running
From: Jiri Pirko @ 2012-06-29  6:41 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: mst, netdev, shimoda.hiroaki, virtualization, danny.kukawka,
	edumazet, davem
In-Reply-To: <1340921854.2577.16.camel@bwh-desktop.uk.solarflarecom.com>

Fri, Jun 29, 2012 at 12:17:34AM CEST, bhutchings@solarflare.com wrote:
>On Thu, 2012-06-28 at 16:10 +0200, Jiri Pirko wrote:
>> Introduce IFF_LIFE_ADDR_CHANGE priv_flag and use it to disable
>> netif_running() check in eth_mac_addr()
>>
>> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
>> ---
>>  include/linux/if.h |    2 ++
>>  net/ethernet/eth.c |    2 +-
>>  2 files changed, 3 insertions(+), 1 deletion(-)
>> 
>> diff --git a/include/linux/if.h b/include/linux/if.h
>> index f995c66..fd9ee7c 100644
>> --- a/include/linux/if.h
>> +++ b/include/linux/if.h
>> @@ -81,6 +81,8 @@
>>  #define IFF_UNICAST_FLT	0x20000		/* Supports unicast filtering	*/
>>  #define IFF_TEAM_PORT	0x40000		/* device used as team port */
>>  #define IFF_SUPP_NOFCS	0x80000		/* device supports sending custom FCS */
>> +#define IFF_LIFE_ADDR_CHANGE 0x100000	/* device supports hardware address
>> +					 * change when it's running */
>[...]
>
>Any device that has IFF_UNICAST_FLT can update the unicast MAC filter
>while it's running; doesn't that go hand-in-hand with being able to
>handle changes to the primary MAC address?  Is the new flag really
>necessary at all?

Hmm, this makes sense. But, can you guarantee that all devices behave like this?

Also, there are many devices that does not support unicast filtering
and yet they support updating mac adress while running.

Jirka

>
>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: linux-next: manual merge of the net-next tree with the net tree
From: Sjur BRENDELAND @ 2012-06-29  6:46 UTC (permalink / raw)
  To: Stephen Rothwell, David Miller, netdev@vger.kernel.org
  Cc: linux-next@vger.kernel.org, linux-kernel@vger.kernel.org,
	Per ELLEFSEN, Kim LILLIESTIERNA
In-Reply-To: <20120626131543.21e4338894b19f5e02f8bdde@canb.auug.org.au>

Hi Stephen,

> Today's linux-next merge of the net-next tree got a conflict in
> drivers/net/caif/caif_hsi.c between commits 3935600a7f34 ("caif-hsi:
> Bugfix - Piggyback'ed embedded CAIF frame lost") and 1fdc7630b2cb
> ("caif-hsi: Add missing return in error path") from the net tree and
> commits 4e7bb59d49fb ("caif-hsi: Removed dead code") and c41254006377
> ("caif-hsi: Add rtnl support") from the net-next tree.
> 
> I fixed them up (see below) and can carry the fix as necessary.

Sorry for late response. Your merge looks perfect.

Thanks,
Sjur

^ permalink raw reply

* Re: [Xen-devel] [PATCH 1/1] xen/netback: only non-freed SKB is queued into tx_queue
From: Ian Campbell @ 2012-06-29  7:23 UTC (permalink / raw)
  To: David Miller
  Cc: annie.li@oracle.com, xen-devel@lists.xensource.com,
	netdev@vger.kernel.org, konrad.wilk@oracle.com,
	kurt.hackel@oracle.com
In-Reply-To: <20120628.165550.1816352825092253548.davem@davemloft.net>

On Fri, 2012-06-29 at 00:55 +0100, David Miller wrote:
> From: annie.li@oracle.com
> Date: Wed, 27 Jun 2012 18:46:58 +0800
> 
> > From: Annie Li <Annie.li@oracle.com>
> > 
> > After SKB is queued into tx_queue, it will be freed if request_gop is NULL.
> > However, no dequeue action is called in this situation, it is likely that
> > tx_queue constains freed SKB. This patch should fix this issue, and it is
> > based on 3.5.0-rc4+.
> > 
> > This issue is found through code inspection, no bug is seen with it currently.
> > I run netperf test for several hours, and no network regression was found.
> > 
> > Signed-off-by: Annie Li <annie.li@oracle.com>
> 
> I lack the expertiece necessary to properly review this, so I really
> need a Xen expert to look this over.

Sorry, I put it to one side waiting for the repost to netdev and then
forgot about it...

Yes, this change looks good to me:

Acked-by: Ian Campbell <ian.campbell@citrix.com>

^ permalink raw reply

* Re: [PATCH net-next] caif-hsi: Fix merge issues.
From: David Miller @ 2012-06-29  7:48 UTC (permalink / raw)
  To: sjur.brandeland; +Cc: netdev, sjurbren
In-Reply-To: <1340951780-27406-1-git-send-email-sjur.brandeland@stericsson.com>

From: sjur.brandeland@stericsson.com
Date: Fri, 29 Jun 2012 08:36:20 +0200

> From: Sjur Brændeland <sjur.brandeland@stericsson.com>
> 
> Fix the failing merge in net-next by reverting the last
> net-next merge for caif_hsi.c and then merge in the commit:
> "caif-hsi: Bugfix - Piggyback'ed embedded CAIF frame lost"
> from the net repository. 
> 
> The commit:"caif-hsi: Add missing return in error path" from
> net repository was dropped, as it changed code previously removed in the 
> net-next repository.
> 
> Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>

Applied, thanks a lot.

^ permalink raw reply

* Re: [Xen-devel] [PATCH 1/1] xen/netback: only non-freed SKB is queued into tx_queue
From: David Miller @ 2012-06-29  7:50 UTC (permalink / raw)
  To: Ian.Campbell; +Cc: annie.li, xen-devel, netdev, konrad.wilk, kurt.hackel
In-Reply-To: <1340954589.5953.12.camel@dagon.hellion.org.uk>

From: Ian Campbell <Ian.Campbell@citrix.com>
Date: Fri, 29 Jun 2012 08:23:09 +0100

> On Fri, 2012-06-29 at 00:55 +0100, David Miller wrote:
>> From: annie.li@oracle.com
>> Date: Wed, 27 Jun 2012 18:46:58 +0800
>> 
>> > From: Annie Li <Annie.li@oracle.com>
>> > 
>> > After SKB is queued into tx_queue, it will be freed if request_gop is NULL.
>> > However, no dequeue action is called in this situation, it is likely that
>> > tx_queue constains freed SKB. This patch should fix this issue, and it is
>> > based on 3.5.0-rc4+.
>> > 
>> > This issue is found through code inspection, no bug is seen with it currently.
>> > I run netperf test for several hours, and no network regression was found.
>> > 
>> > Signed-off-by: Annie Li <annie.li@oracle.com>
>> 
>> I lack the expertiece necessary to properly review this, so I really
>> need a Xen expert to look this over.
> 
> Sorry, I put it to one side waiting for the repost to netdev and then
> forgot about it...
> 
> Yes, this change looks good to me:
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>

Thanks, applied to net-next.

^ permalink raw reply

* Re: [PATCH] ipv6_tunnel: Allow receiving packets on the fallback tunnel if they pass sanity checks
From: David Miller @ 2012-06-29  7:52 UTC (permalink / raw)
  To: phil; +Cc: netdev, phild, ville.nuorvala
In-Reply-To: <20120629041552.GA27362@ipom.com>

From: Phil Dibowitz <phil@ipom.com>
Date: Thu, 28 Jun 2012 21:15:52 -0700

> From: Ville Nuorvala <ville.nuorvala@gmail.com>
> 
> At Facebook, we do Layer-3 DSR via IP-in-IP tunneling. Our load balancers wrap
> an extra IP header on incoming packets so they can be routed to the backend.
> In the v4 tunnel driver, when these packets fall on the default tunl0 device,
> the behavior is to decapsulate them and drop them back on the stack. So our
> setup is that tunl0 has the VIP and eth0 has (obviously) the backend's real
> address.
> 
> In IPv6 we do the same thing, but the v6 tunnel driver didn't have this same
> behavior - if you didn't have an explicit tunnel setup, it would drop the
> packet.
> 
> This patch brings that v4 feature to the v6 driver.
> 
> The same IPv6 address checks are performed as with any normal tunnel,
> but as the fallback tunnel endpoint addresses are unspecified, the checks
> must be performed on a per-packet basis, rather than at tunnel
> configuration time.
> 
> [Patch description modified by phil@ipom.com]
> 
> Signed-off-by: Ville Nuorvala <ville.nuorvala@gmail.com>
> Tested-by: Phil Dibowitz <phil@ipom.com>

Applied to net-next

^ permalink raw reply

* Re: [PATCH net-next] fq_codel: report congestion notification at enqueue time
From: David Miller @ 2012-06-29  7:53 UTC (permalink / raw)
  To: eric.dumazet; +Cc: nanditad, netdev, codel, ycheng, ncardwell, mattmathis
In-Reply-To: <1340949008.29822.73.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 29 Jun 2012 07:50:08 +0200

> Hmm, problem is the sender thinks the packet was queued for
> transmission.
> 
>         ret = macvlan_queue_xmit(skb, dev);
>         if (likely(ret == NET_XMIT_SUCCESS || ret == NET_XMIT_CN)) {
>                 struct macvlan_pcpu_stats *pcpu_stats;
> 
>                 pcpu_stats = this_cpu_ptr(vlan->pcpu_stats);
>                 u64_stats_update_begin(&pcpu_stats->syncp);
>                 pcpu_stats->tx_packets++;
>                 pcpu_stats->tx_bytes += len;
>                 u64_stats_update_end(&pcpu_stats->syncp);
>         } else {
>                 this_cpu_inc(vlan->pcpu_stats->tx_dropped);
>         }
> 
> NET_XMIT_CN has a lazy semantic it seems.
> 
> I will just dont rely on it.

I think we cannot just ignore this issue.  I will take a deeper look,
because we should have NET_XMIT_CN be very well defined and adjust any
mis-use.

^ permalink raw reply

* Re: [PATCH net-next] net: l2tp_eth: provide tx_dropped counter
From: David Miller @ 2012-06-29  7:54 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1340950513.29822.103.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 29 Jun 2012 08:15:13 +0200

> From: Eric Dumazet <edumazet@google.com>
> 
> Change l2tp_xmit_skb() to return NET_XMIT_DROP in case skb is dropped.
> 
> Use kfree_skb() instead dev_kfree_skb() for drop_monitor pleasure.
> 
> Support tx_dropped counter for l2tp_eth
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied.

^ 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