Netdev List
 help / color / mirror / Atom feed
* [PATCH 4/7] netfilter: ipset: Consistent userspace testing with nomatch flag
From: Pablo Neira Ayuso @ 2013-09-17 22:07 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1379455654-3905-1-git-send-email-pablo@netfilter.org>

From: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>

The "nomatch" commandline flag should invert the matching at testing,
similarly to the --return-nomatch flag of the "set" match of iptables.
Until now it worked with the elements with "nomatch" flag only. From
now on it works with elements without the flag too, i.e:

 # ipset n test hash:net
 # ipset a test 10.0.0.0/24 nomatch
 # ipset t test 10.0.0.1
 10.0.0.1 is NOT in set test.
 # ipset t test 10.0.0.1 nomatch
 10.0.0.1 is in set test.

 # ipset a test 192.168.0.0/24
 # ipset t test 192.168.0.1
 192.168.0.1 is in set test.
 # ipset t test 192.168.0.1 nomatch
 192.168.0.1 is NOT in set test.

 Before the patch the results were

 ...
 # ipset t test 192.168.0.1
 192.168.0.1 is in set test.
 # ipset t test 192.168.0.1 nomatch
 192.168.0.1 is in set test.

Signed-off-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
---
 include/linux/netfilter/ipset/ip_set.h      |    6 ++++--
 net/netfilter/ipset/ip_set_core.c           |    3 +--
 net/netfilter/ipset/ip_set_hash_ipportnet.c |    4 ++--
 net/netfilter/ipset/ip_set_hash_net.c       |    4 ++--
 net/netfilter/ipset/ip_set_hash_netiface.c  |    4 ++--
 net/netfilter/ipset/ip_set_hash_netport.c   |    4 ++--
 6 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/include/linux/netfilter/ipset/ip_set.h b/include/linux/netfilter/ipset/ip_set.h
index d80e275..9ac9fbd 100644
--- a/include/linux/netfilter/ipset/ip_set.h
+++ b/include/linux/netfilter/ipset/ip_set.h
@@ -296,10 +296,12 @@ ip_set_eexist(int ret, u32 flags)
 
 /* Match elements marked with nomatch */
 static inline bool
-ip_set_enomatch(int ret, u32 flags, enum ipset_adt adt)
+ip_set_enomatch(int ret, u32 flags, enum ipset_adt adt, struct ip_set *set)
 {
 	return adt == IPSET_TEST &&
-	       ret == -ENOTEMPTY && ((flags >> 16) & IPSET_FLAG_NOMATCH);
+	       (set->type->features & IPSET_TYPE_NOMATCH) &&
+	       ((flags >> 16) & IPSET_FLAG_NOMATCH) &&
+	       (ret > 0 || ret == -ENOTEMPTY);
 }
 
 /* Check the NLA_F_NET_BYTEORDER flag */
diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
index f771390..c8c303c 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -1489,8 +1489,7 @@ ip_set_utest(struct sock *ctnl, struct sk_buff *skb,
 	if (ret == -EAGAIN)
 		ret = 1;
 
-	return (ret < 0 && ret != -ENOTEMPTY) ? ret :
-		ret > 0 ? 0 : -IPSET_ERR_EXIST;
+	return ret > 0 ? 0 : -IPSET_ERR_EXIST;
 }
 
 /* Get headed data of a set */
diff --git a/net/netfilter/ipset/ip_set_hash_ipportnet.c b/net/netfilter/ipset/ip_set_hash_ipportnet.c
index c6a5253..f15f3e2 100644
--- a/net/netfilter/ipset/ip_set_hash_ipportnet.c
+++ b/net/netfilter/ipset/ip_set_hash_ipportnet.c
@@ -260,7 +260,7 @@ hash_ipportnet4_uadt(struct ip_set *set, struct nlattr *tb[],
 		e.ip = htonl(ip);
 		e.ip2 = htonl(ip2_from & ip_set_hostmask(e.cidr + 1));
 		ret = adtfn(set, &e, &ext, &ext, flags);
-		return ip_set_enomatch(ret, flags, adt) ? 1 :
+		return ip_set_enomatch(ret, flags, adt, set) ? -ret :
 		       ip_set_eexist(ret, flags) ? 0 : ret;
 	}
 
@@ -544,7 +544,7 @@ hash_ipportnet6_uadt(struct ip_set *set, struct nlattr *tb[],
 
 	if (adt == IPSET_TEST || !with_ports || !tb[IPSET_ATTR_PORT_TO]) {
 		ret = adtfn(set, &e, &ext, &ext, flags);
-		return ip_set_enomatch(ret, flags, adt) ? 1 :
+		return ip_set_enomatch(ret, flags, adt, set) ? -ret :
 		       ip_set_eexist(ret, flags) ? 0 : ret;
 	}
 
diff --git a/net/netfilter/ipset/ip_set_hash_net.c b/net/netfilter/ipset/ip_set_hash_net.c
index da740ce..223e9f5 100644
--- a/net/netfilter/ipset/ip_set_hash_net.c
+++ b/net/netfilter/ipset/ip_set_hash_net.c
@@ -199,7 +199,7 @@ hash_net4_uadt(struct ip_set *set, struct nlattr *tb[],
 	if (adt == IPSET_TEST || !tb[IPSET_ATTR_IP_TO]) {
 		e.ip = htonl(ip & ip_set_hostmask(e.cidr));
 		ret = adtfn(set, &e, &ext, &ext, flags);
-		return ip_set_enomatch(ret, flags, adt) ? 1 :
+		return ip_set_enomatch(ret, flags, adt, set) ? -ret:
 		       ip_set_eexist(ret, flags) ? 0 : ret;
 	}
 
@@ -396,7 +396,7 @@ hash_net6_uadt(struct ip_set *set, struct nlattr *tb[],
 
 	ret = adtfn(set, &e, &ext, &ext, flags);
 
-	return ip_set_enomatch(ret, flags, adt) ? 1 :
+	return ip_set_enomatch(ret, flags, adt, set) ? -ret :
 	       ip_set_eexist(ret, flags) ? 0 : ret;
 }
 
diff --git a/net/netfilter/ipset/ip_set_hash_netiface.c b/net/netfilter/ipset/ip_set_hash_netiface.c
index 84ae6f6..7d798d5 100644
--- a/net/netfilter/ipset/ip_set_hash_netiface.c
+++ b/net/netfilter/ipset/ip_set_hash_netiface.c
@@ -368,7 +368,7 @@ hash_netiface4_uadt(struct ip_set *set, struct nlattr *tb[],
 	if (adt == IPSET_TEST || !tb[IPSET_ATTR_IP_TO]) {
 		e.ip = htonl(ip & ip_set_hostmask(e.cidr));
 		ret = adtfn(set, &e, &ext, &ext, flags);
-		return ip_set_enomatch(ret, flags, adt) ? 1 :
+		return ip_set_enomatch(ret, flags, adt, set) ? -ret :
 		       ip_set_eexist(ret, flags) ? 0 : ret;
 	}
 
@@ -634,7 +634,7 @@ hash_netiface6_uadt(struct ip_set *set, struct nlattr *tb[],
 
 	ret = adtfn(set, &e, &ext, &ext, flags);
 
-	return ip_set_enomatch(ret, flags, adt) ? 1 :
+	return ip_set_enomatch(ret, flags, adt, set) ? -ret :
 	       ip_set_eexist(ret, flags) ? 0 : ret;
 }
 
diff --git a/net/netfilter/ipset/ip_set_hash_netport.c b/net/netfilter/ipset/ip_set_hash_netport.c
index 9a08698..09d6690 100644
--- a/net/netfilter/ipset/ip_set_hash_netport.c
+++ b/net/netfilter/ipset/ip_set_hash_netport.c
@@ -244,7 +244,7 @@ hash_netport4_uadt(struct ip_set *set, struct nlattr *tb[],
 	if (adt == IPSET_TEST || !(with_ports || tb[IPSET_ATTR_IP_TO])) {
 		e.ip = htonl(ip & ip_set_hostmask(e.cidr + 1));
 		ret = adtfn(set, &e, &ext, &ext, flags);
-		return ip_set_enomatch(ret, flags, adt) ? 1 :
+		return ip_set_enomatch(ret, flags, adt, set) ? -ret :
 		       ip_set_eexist(ret, flags) ? 0 : ret;
 	}
 
@@ -489,7 +489,7 @@ hash_netport6_uadt(struct ip_set *set, struct nlattr *tb[],
 
 	if (adt == IPSET_TEST || !with_ports || !tb[IPSET_ATTR_PORT_TO]) {
 		ret = adtfn(set, &e, &ext, &ext, flags);
-		return ip_set_enomatch(ret, flags, adt) ? 1 :
+		return ip_set_enomatch(ret, flags, adt, set) ? -ret :
 		       ip_set_eexist(ret, flags) ? 0 : ret;
 	}
 
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 2/7] netfilter: nf_nat_proto_icmpv6:: fix wrong comparison in icmpv6_manip_pkt
From: Pablo Neira Ayuso @ 2013-09-17 22:07 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1379455654-3905-1-git-send-email-pablo@netfilter.org>

From: Phil Oester <kernel@linuxace.com>

In commit 58a317f1 (netfilter: ipv6: add IPv6 NAT support), icmpv6_manip_pkt
was added with an incorrect comparison of ICMP codes to types.  This causes
problems when using NAT rules with the --random option.  Correct the
comparison.

This closes netfilter bugzilla #851, reported by Alexander Neumann.

Signed-off-by: Phil Oester <kernel@linuxace.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/ipv6/netfilter/nf_nat_proto_icmpv6.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/netfilter/nf_nat_proto_icmpv6.c b/net/ipv6/netfilter/nf_nat_proto_icmpv6.c
index 61aaf70..2205e8e 100644
--- a/net/ipv6/netfilter/nf_nat_proto_icmpv6.c
+++ b/net/ipv6/netfilter/nf_nat_proto_icmpv6.c
@@ -69,8 +69,8 @@ icmpv6_manip_pkt(struct sk_buff *skb,
 	hdr = (struct icmp6hdr *)(skb->data + hdroff);
 	l3proto->csum_update(skb, iphdroff, &hdr->icmp6_cksum,
 			     tuple, maniptype);
-	if (hdr->icmp6_code == ICMPV6_ECHO_REQUEST ||
-	    hdr->icmp6_code == ICMPV6_ECHO_REPLY) {
+	if (hdr->icmp6_type == ICMPV6_ECHO_REQUEST ||
+	    hdr->icmp6_type == ICMPV6_ECHO_REPLY) {
 		inet_proto_csum_replace2(&hdr->icmp6_cksum, skb,
 					 hdr->icmp6_identifier,
 					 tuple->src.u.icmp.id, 0);
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 6/7] netfilter: ipset: Fix serious failure in CIDR tracking
From: Pablo Neira Ayuso @ 2013-09-17 22:07 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1379455654-3905-1-git-send-email-pablo@netfilter.org>

From: Oliver Smith <oliver@8.c.9.b.0.7.4.0.1.0.0.2.ip6.arpa>

This fixes a serious bug affecting all hash types with a net element -
specifically, if a CIDR value is deleted such that none of the same size
exist any more, all larger (less-specific) values will then fail to
match. Adding back any prefix with a CIDR equal to or more specific than
the one deleted will fix it.

Steps to reproduce:
ipset -N test hash:net
ipset -A test 1.1.0.0/16
ipset -A test 2.2.2.0/24
ipset -T test 1.1.1.1           #1.1.1.1 IS in set
ipset -D test 2.2.2.0/24
ipset -T test 1.1.1.1           #1.1.1.1 IS NOT in set

This is due to the fact that the nets counter was unconditionally
decremented prior to the iteration that shifts up the entries. Now, we
first check if there is a proceeding entry and if not, decrement it and
return. Otherwise, we proceed to iterate and then zero the last element,
which, in most cases, will already be zero.

Signed-off-by: Oliver Smith <oliver@8.c.9.b.0.7.4.0.1.0.0.2.ip6.arpa>
Signed-off-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
---
 net/netfilter/ipset/ip_set_hash_gen.h |   28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/net/netfilter/ipset/ip_set_hash_gen.h b/net/netfilter/ipset/ip_set_hash_gen.h
index 57beb17..707bc52 100644
--- a/net/netfilter/ipset/ip_set_hash_gen.h
+++ b/net/netfilter/ipset/ip_set_hash_gen.h
@@ -325,18 +325,22 @@ mtype_add_cidr(struct htype *h, u8 cidr, u8 nets_length)
 static void
 mtype_del_cidr(struct htype *h, u8 cidr, u8 nets_length)
 {
-	u8 i, j;
-
-	for (i = 0; i < nets_length - 1 && h->nets[i].cidr != cidr; i++)
-		;
-	h->nets[i].nets--;
-
-	if (h->nets[i].nets != 0)
-		return;
-
-	for (j = i; j < nets_length - 1 && h->nets[j].nets; j++) {
-		h->nets[j].cidr = h->nets[j + 1].cidr;
-		h->nets[j].nets = h->nets[j + 1].nets;
+	u8 i, j, net_end = nets_length - 1;
+
+	for (i = 0; i < nets_length; i++) {
+	        if (h->nets[i].cidr != cidr)
+	                continue;
+                if (h->nets[i].nets > 1 || i == net_end ||
+                    h->nets[i + 1].nets == 0) {
+                        h->nets[i].nets--;
+                        return;
+                }
+                for (j = i; j < net_end && h->nets[j].nets; j++) {
+		        h->nets[j].cidr = h->nets[j + 1].cidr;
+		        h->nets[j].nets = h->nets[j + 1].nets;
+                }
+                h->nets[j].nets = 0;
+                return;
 	}
 }
 #endif
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 5/7] netfilter: ipset: Validate the set family and not the set type family at swapping
From: Pablo Neira Ayuso @ 2013-09-17 22:07 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1379455654-3905-1-git-send-email-pablo@netfilter.org>

From: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>

This closes netfilter bugzilla #843, reported by Quentin Armitage.

Signed-off-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
---
 net/netfilter/ipset/ip_set_core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
index c8c303c..f2e30fb 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -1052,7 +1052,7 @@ ip_set_swap(struct sock *ctnl, struct sk_buff *skb,
 	 * Not an artificial restriction anymore, as we must prevent
 	 * possible loops created by swapping in setlist type of sets. */
 	if (!(from->type->features == to->type->features &&
-	      from->type->family == to->type->family))
+	      from->family == to->family))
 		return -IPSET_ERR_TYPE_MISMATCH;
 
 	strncpy(from_name, from->name, IPSET_MAXNAMELEN);
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 7/7] netfilter: nfnetlink_queue: use network skb for sequence adjustment
From: Pablo Neira Ayuso @ 2013-09-17 22:07 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1379455654-3905-1-git-send-email-pablo@netfilter.org>

From: Gao feng <gaofeng@cn.fujitsu.com>

Instead of the netlink skb.

Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nfnetlink_queue_core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/nfnetlink_queue_core.c b/net/netfilter/nfnetlink_queue_core.c
index 95a98c8..ae2e5c1 100644
--- a/net/netfilter/nfnetlink_queue_core.c
+++ b/net/netfilter/nfnetlink_queue_core.c
@@ -1009,7 +1009,7 @@ nfqnl_recv_verdict(struct sock *ctnl, struct sk_buff *skb,
 			verdict = NF_DROP;
 
 		if (ct)
-			nfqnl_ct_seq_adjust(skb, ct, ctinfo, diff);
+			nfqnl_ct_seq_adjust(entry->skb, ct, ctinfo, diff);
 	}
 
 	if (nfqa[NFQA_MARK])
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 3/7] netfilter: ipset: Skip really non-first fragments for IPv6 when getting port/protocol
From: Pablo Neira Ayuso @ 2013-09-17 22:07 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1379455654-3905-1-git-send-email-pablo@netfilter.org>

From: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>

Signed-off-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
---
 net/netfilter/ipset/ip_set_getport.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/ipset/ip_set_getport.c b/net/netfilter/ipset/ip_set_getport.c
index 6fdf88a..dac156f 100644
--- a/net/netfilter/ipset/ip_set_getport.c
+++ b/net/netfilter/ipset/ip_set_getport.c
@@ -116,12 +116,12 @@ ip_set_get_ip6_port(const struct sk_buff *skb, bool src,
 {
 	int protoff;
 	u8 nexthdr;
-	__be16 frag_off;
+	__be16 frag_off = 0;
 
 	nexthdr = ipv6_hdr(skb)->nexthdr;
 	protoff = ipv6_skip_exthdr(skb, sizeof(struct ipv6hdr), &nexthdr,
 				   &frag_off);
-	if (protoff < 0)
+	if (protoff < 0 || (frag_off & htons(~0x7)) != 0)
 		return false;
 
 	return get_port(skb, nexthdr, protoff, src, port, proto);
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 1/7] netfilter: nf_conntrack: use RCU safe kfree for conntrack extensions
From: Pablo Neira Ayuso @ 2013-09-17 22:07 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1379455654-3905-1-git-send-email-pablo@netfilter.org>

From: Michal Kubeček <mkubecek@suse.cz>

Commit 68b80f11 (netfilter: nf_nat: fix RCU races) introduced
RCU protection for freeing extension data when reallocation
moves them to a new location. We need the same protection when
freeing them in nf_ct_ext_free() in order to prevent a
use-after-free by other threads referencing a NAT extension data
via bysource list.

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/netfilter/nf_conntrack_extend.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/net/netfilter/nf_conntrack_extend.h b/include/net/netfilter/nf_conntrack_extend.h
index ff95434..88a1d40 100644
--- a/include/net/netfilter/nf_conntrack_extend.h
+++ b/include/net/netfilter/nf_conntrack_extend.h
@@ -86,7 +86,7 @@ static inline void nf_ct_ext_destroy(struct nf_conn *ct)
 static inline void nf_ct_ext_free(struct nf_conn *ct)
 {
 	if (ct->ext)
-		kfree(ct->ext);
+		kfree_rcu(ct->ext, rcu);
 }
 
 /* Add this type, returns pointer to data or NULL. */
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* ath9k: remove self-assignment in ath9k_eeprom_request
From: Dave Jones @ 2013-09-17 21:35 UTC (permalink / raw)
  To: netdev; +Cc: mcgrof, linville

Assigning a variable to itself is pointless, moreso when it's overwritten immediately.

Introduced in ab5c4f71d8c7add173a2d32e5beefdaaf1b7cbbc
 ("ath9k: allow to load EEPROM content via firmware API")

Signed-off-by: Dave Jones <davej@fedoraproject.org>

diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index 9a1f349..0cb3f2d 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -572,7 +572,7 @@ static void ath9k_eeprom_request_cb(const struct firmware *eeprom_blob,
 static int ath9k_eeprom_request(struct ath_softc *sc, const char *name)
 {
 	struct ath9k_eeprom_ctx ec;
-	struct ath_hw *ah = ah = sc->sc_ah;
+	struct ath_hw *ah = sc->sc_ah;
 	int err;
 
 	/* try to load the EEPROM content asynchronously */

^ permalink raw reply related

* [PATCH] p54usb: fix leak at failure path in p54u_load_firmware()
From: Alexey Khoroshilov @ 2013-09-17 20:57 UTC (permalink / raw)
  To: Christian Lamparter
  Cc: Alexey Khoroshilov, John W. Linville, linux-wireless, netdev,
	linux-kernel, ldv-project

If request_firmware_nowait() fails in p54u_load_firmware(),
p54u_load_firmware_cb is not called and no one decrements usb_dev refcnt.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
 drivers/net/wireless/p54/p54usb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/p54/p54usb.c b/drivers/net/wireless/p54/p54usb.c
index b9deef6..7fa81d1 100644
--- a/drivers/net/wireless/p54/p54usb.c
+++ b/drivers/net/wireless/p54/p54usb.c
@@ -979,6 +979,7 @@ static int p54u_load_firmware(struct ieee80211_hw *dev,
 	if (err) {
 		dev_err(&priv->udev->dev, "(p54usb) cannot load firmware %s "
 					  "(%d)!\n", p54u_fwlist[i].fw, err);
+		usb_put_dev(udev);
 	}
 
 	return err;
-- 
1.8.1.2

^ permalink raw reply related

* Re: [PATCH 1/2] Re: net, vxlan Fix compile warning
From: Ben Hutchings @ 2013-09-17 20:26 UTC (permalink / raw)
  To: Prarit Bhargava; +Cc: netdev, jpirko, davem, stephen
In-Reply-To: <1379445167-28488-1-git-send-email-prarit@redhat.com>

You don't seem to have completed this change:

On Tue, 2013-09-17 at 15:12 -0400, Prarit Bhargava wrote:
[...]
> @@ -2291,9 +2289,9 @@ static struct vxlan_sock *vxlan_socket_create(struct net *net, __be16 port,
>  	INIT_WORK(&vs->del_work, vxlan_del_work);
>  
>  	if (ipv6)
> -		rc = create_v6_sock(net, port, &sock);
> +		sock = create_v6_sock(net, port);
>  	else
> -		rc = create_v4_sock(net, port, &sock);
> +		sock = create_v4_sock(net, port);
>  	if (rc < 0) {

	if (IS_ERR(sock)) {

>  		kfree(vs);
>  		return ERR_PTR(rc);

		return ERR_CAST(sock);

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

* ip6_finish_output2 change broke netfilter xt_TEE target
From: Phil Oester @ 2013-09-17 19:54 UTC (permalink / raw)
  To: yoshfuji; +Cc: netdev, netfilter-devel

The change made in commit 6fd6ce20 (ipv6: Do not depend on rt->n in
ip6_finish_output2) broke the xt_TEE target for IPv6 packets.  Instead
of using the nexthop provided in the --gateway option, ip6_finish_output2
is now performing neighbor solicitation for the original daddr in the
copied skb.

Similar breakage occurred in IPv4, and was fixed (in 2ad5b9e4) by using
the flag FLOWI_FLAG_KNOWN_NH.  I can find no easy way to make use of that
flag here.  Reverting 6fd6ce20 makes TEE work again, but I am not clear
on what problem that commit was attempting to solve.  Yoshifuji?

Phil


^ permalink raw reply

* Re: [PATCH] vhost/scsi: use vmalloc for order-10 allocation
From: Michael S. Tsirkin @ 2013-09-17 19:51 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: kvm, netdev, linux-kernel, virtualization, Dan Aloni
In-Reply-To: <52389C0D.7070307@cogentembedded.com>

On Tue, Sep 17, 2013 at 10:14:37PM +0400, Sergei Shtylyov wrote:
> Hello.
> 
> On 09/17/2013 11:21 AM, Michael S. Tsirkin wrote:
> 
> >As vhost scsi device struct is large, if the device is
> >created on a busy system, kzalloc() might fail, so this patch does a
> >fallback to vzalloc().
> 
> >As vmalloc() adds overhead on data-path, add __GFP_REPEAT
> >to kzalloc() flags to do this fallback only when really needed.
> 
> >Reported-by: Dan Aloni <alonid@stratoscale.com>
> >Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >---
> 
> >I put this on my vhost fixes branch, intend to merge for 3.12.
> >Dan, could you please confirm this works for you?
> 
> >  drivers/vhost/scsi.c | 41 +++++++++++++++++++++++++++--------------
> >  1 file changed, 27 insertions(+), 14 deletions(-)
> 
> >diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> >index 4b79a1f..2c30bb0 100644
> >--- a/drivers/vhost/scsi.c
> >+++ b/drivers/vhost/scsi.c
> >@@ -1373,21 +1373,30 @@ static int vhost_scsi_set_features(struct vhost_scsi *vs, u64 features)
> >  	return 0;
> >  }
> >
> >+static void vhost_scsi_free(struct vhost_scsi *vs)
> >+{
> >+        if (is_vmalloc_addr(vs))
> >+                vfree(vs);
> >+        else
> >+                kfree(vs);
> 
>    Indent with the tabs ISO spaces, please.
> 
> >+}
> >+
> >  static int vhost_scsi_open(struct inode *inode, struct file *f)
> >  {
> >  	struct vhost_scsi *vs;
> >  	struct vhost_virtqueue **vqs;
> >-	int r, i;
> >+	int r = -ENOMEM, i;
> >
> >-	vs = kzalloc(sizeof(*vs), GFP_KERNEL);
> >-	if (!vs)
> >-		return -ENOMEM;
> >+        vs = kzalloc(sizeof(*vs), GFP_KERNEL | __GFP_NOWARN | __GFP_REPEAT);
> 
>    Indent here with a tab, please.
> 
> >+	if (!vs) {
> >+		vs = vzalloc(sizeof(*vs));
> >+		if (!vs)
> >+			goto err_vs;
> >+	}
> 
> WBR, Sergei

Thanks, I'll fix this up.

^ permalink raw reply

* Re: [PATCH] sh_eth: call phy_scan_fixups() after PHY reset
From: David Miller @ 2013-09-17 19:44 UTC (permalink / raw)
  To: sergei.shtylyov
  Cc: netdev, nobuhiro.iwamatsu.yj, linux-sh, laurent.pinchart+renesas
In-Reply-To: <201309140410.38396.sergei.shtylyov@cogentembedded.com>

From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Sat, 14 Sep 2013 04:10:37 +0400

> Sometimes the PHY reset that sh_eth_phy_start() does effects the PHY registers
> registers values of which are vital for the correct functioning of the driver.
> Unfortunately, the existing PHY platform fixup mechanism doesn't help  here as
> it only hooks PHY resets done by ioctl() calls. Calling phy_scan_fixups() from
> the driver helps here. With a proper platform fixup, this fixes NFS timeouts on
> the SH-Mobile Lager board. 
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

The PHY layer is designed to naturally already take care of this kind of
thing.  I think that part of the problem is that you're fighting the
natural control flow the PHY layer provides.

When the phy_connect() is performed, what we end up doing is calling
phy_attach_direct() which invokes the ->probe() method of the driver
and then afterwards we do phy_init_hw() which takes care of doing
the fixup calls.

So if you really need to do a BMCR reset then run the fixups I'd like
you to look into making that happen within the provided control
flow rather than with an exceptional explicit call to run the fixups.

I'm willing to be convinced that this is a better or necessary approach
but you'll need to explain it to me.

Thanks.

^ permalink raw reply

* Re: [PATCH 2/2] Re: net, mellanox mlx4 Fix compile warnings
From: David Miller @ 2013-09-17 19:28 UTC (permalink / raw)
  To: or.gerlitz; +Cc: prarit, jackm, netdev, dledford, amirv, ogerlitz
In-Reply-To: <CAJZOPZJ5kBW3_NQiw3S-DvDkYonzrW-EgxXdAqRcS1KGUrQu=w@mail.gmail.com>

From: Or Gerlitz <or.gerlitz@gmail.com>
Date: Tue, 17 Sep 2013 22:25:57 +0300

> On Tue, Sep 17, 2013 at 10:13 PM, Prarit Bhargava <prarit@redhat.com> wrote:
>> Fix unitialized variable warnings.
>>
>> drivers/net/ethernet/mellanox/mlx4/resource_tracker.c: In function ‘mlx4_HW2SW_CQ_wrapper’:
>> drivers/net/ethernet/mellanox/mlx4/resource_tracker.c:2551:16: error: ‘cq’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>>   atomic_dec(&cq->mtt->ref_count);
>>                 ^
>> drivers/net/ethernet/mellanox/mlx4/resource_tracker.c: In function ‘mlx4_HW2SW_SRQ_wrapper’:
>> drivers/net/ethernet/mellanox/mlx4/resource_tracker.c:2734:17: error: ‘srq’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>>   atomic_dec(&srq->mtt->ref_count);
>>
>> [v2]: davem suggested making cq_res_start_move_to() return 'cq' as an error
>> pointer instead of setting 'cq' by reference.  I also did the same for srq.
> 
> Pravit, as I wrote you earlier on this thread, Jack from our team
> maintains this piece (SRIOV resource tracker) of the mlx4 core driver
> code, so I am adding him. Dave, as many of us here he might be OOO for
> the coming ten days for holiday vacation, so would ask you to please
> wait patiently for his ack/nak...

I already stated that I wanted this issue fixed differently.

Specifically I said that these functions should return error pointers,
instead of trying to return an integer error whilst setting the cq
pointer by reference.

This is a method which is idiomatic and common across the kernel, and
is the preferred way to handle this kind of situation.

^ permalink raw reply

* Re: [PATCH 2/2] Re: net, mellanox mlx4 Fix compile warnings
From: Or Gerlitz @ 2013-09-17 19:25 UTC (permalink / raw)
  To: Prarit Bhargava, Jack Morgenstein
  Cc: netdev@vger.kernel.org, Doug Ledford, Amir Vadai, David Miller,
	Or Gerlitz
In-Reply-To: <1379445223-28550-1-git-send-email-prarit@redhat.com>

On Tue, Sep 17, 2013 at 10:13 PM, Prarit Bhargava <prarit@redhat.com> wrote:
> Fix unitialized variable warnings.
>
> drivers/net/ethernet/mellanox/mlx4/resource_tracker.c: In function ‘mlx4_HW2SW_CQ_wrapper’:
> drivers/net/ethernet/mellanox/mlx4/resource_tracker.c:2551:16: error: ‘cq’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>   atomic_dec(&cq->mtt->ref_count);
>                 ^
> drivers/net/ethernet/mellanox/mlx4/resource_tracker.c: In function ‘mlx4_HW2SW_SRQ_wrapper’:
> drivers/net/ethernet/mellanox/mlx4/resource_tracker.c:2734:17: error: ‘srq’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>   atomic_dec(&srq->mtt->ref_count);
>
> [v2]: davem suggested making cq_res_start_move_to() return 'cq' as an error
> pointer instead of setting 'cq' by reference.  I also did the same for srq.

Pravit, as I wrote you earlier on this thread, Jack from our team
maintains this piece (SRIOV resource tracker) of the mlx4 core driver
code, so I am adding him. Dave, as many of us here he might be OOO for
the coming ten days for holiday vacation, so would ask you to please
wait patiently for his ack/nak...

Or.

> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> Cc: dledford@redhat.com
> Cc: amirv@mellanox.com
> Cc: davem@davemloft.net
> Cc: ogerlitz@mellanox.com
> ---
>  .../net/ethernet/mellanox/mlx4/resource_tracker.c  |   46 ++++++++++----------
>  1 file changed, 24 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
> index dd68763..343206b 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
> @@ -1073,8 +1073,9 @@ static int eq_res_start_move_to(struct mlx4_dev *dev, int slave, int index,
>         return err;
>  }
>
> -static int cq_res_start_move_to(struct mlx4_dev *dev, int slave, int cqn,
> -                               enum res_cq_states state, struct res_cq **cq)
> +static struct res_cq *cq_res_start_move_to(struct mlx4_dev *dev,
> +                                                 int slave, int cqn,
> +                                                 enum res_cq_states state)
>  {
>         struct mlx4_priv *priv = mlx4_priv(dev);
>         struct mlx4_resource_tracker *tracker = &priv->mfunc.master.res_tracker;
> @@ -1117,18 +1118,19 @@ static int cq_res_start_move_to(struct mlx4_dev *dev, int slave, int cqn,
>                         r->com.from_state = r->com.state;
>                         r->com.to_state = state;
>                         r->com.state = RES_CQ_BUSY;
> -                       if (cq)
> -                               *cq = r;
> +               } else {
> +                       r = ERR_PTR(err);
>                 }
>         }
>
>         spin_unlock_irq(mlx4_tlock(dev));
>
> -       return err;
> +       return r;
>  }
>
> -static int srq_res_start_move_to(struct mlx4_dev *dev, int slave, int index,
> -                                enum res_cq_states state, struct res_srq **srq)
> +static struct res_srq *srq_res_start_move_to(struct mlx4_dev *dev, int slave,
> +                                            int index,
> +                                            enum res_cq_states state)
>  {
>         struct mlx4_priv *priv = mlx4_priv(dev);
>         struct mlx4_resource_tracker *tracker = &priv->mfunc.master.res_tracker;
> @@ -1167,14 +1169,14 @@ static int srq_res_start_move_to(struct mlx4_dev *dev, int slave, int index,
>                         r->com.from_state = r->com.state;
>                         r->com.to_state = state;
>                         r->com.state = RES_SRQ_BUSY;
> -                       if (srq)
> -                               *srq = r;
> +               } else {
> +                       r = ERR_PTR(err);
>                 }
>         }
>
>         spin_unlock_irq(mlx4_tlock(dev));
>
> -       return err;
> +       return r;
>  }
>
>  static void res_abort_move(struct mlx4_dev *dev, int slave,
> @@ -2530,9 +2532,9 @@ int mlx4_SW2HW_CQ_wrapper(struct mlx4_dev *dev, int slave,
>         struct res_cq *cq;
>         struct res_mtt *mtt;
>
> -       err = cq_res_start_move_to(dev, slave, cqn, RES_CQ_HW, &cq);
> -       if (err)
> -               return err;
> +       cq = cq_res_start_move_to(dev, slave, cqn, RES_CQ_HW);
> +       if (IS_ERR(cq))
> +               return PTR_ERR(cq);
>         err = get_res(dev, slave, mtt_base, RES_MTT, &mtt);
>         if (err)
>                 goto out_move;
> @@ -2565,9 +2567,9 @@ int mlx4_HW2SW_CQ_wrapper(struct mlx4_dev *dev, int slave,
>         int cqn = vhcr->in_modifier;
>         struct res_cq *cq;
>
> -       err = cq_res_start_move_to(dev, slave, cqn, RES_CQ_ALLOCATED, &cq);
> -       if (err)
> -               return err;
> +       cq = cq_res_start_move_to(dev, slave, cqn, RES_CQ_ALLOCATED);
> +       if (IS_ERR(cq))
> +               return PTR_ERR(cq);
>         err = mlx4_DMA_wrapper(dev, slave, vhcr, inbox, outbox, cmd);
>         if (err)
>                 goto out_move;
> @@ -2709,9 +2711,9 @@ int mlx4_SW2HW_SRQ_wrapper(struct mlx4_dev *dev, int slave,
>         if (srqn != (be32_to_cpu(srqc->state_logsize_srqn) & 0xffffff))
>                 return -EINVAL;
>
> -       err = srq_res_start_move_to(dev, slave, srqn, RES_SRQ_HW, &srq);
> -       if (err)
> -               return err;
> +       srq = srq_res_start_move_to(dev, slave, srqn, RES_SRQ_ALLOCATED);
> +       if (IS_ERR(srq))
> +               return PTR_ERR(srq);
>         err = get_res(dev, slave, mtt_base, RES_MTT, &mtt);
>         if (err)
>                 goto ex_abort;
> @@ -2748,9 +2750,9 @@ int mlx4_HW2SW_SRQ_wrapper(struct mlx4_dev *dev, int slave,
>         int srqn = vhcr->in_modifier;
>         struct res_srq *srq;
>
> -       err = srq_res_start_move_to(dev, slave, srqn, RES_SRQ_ALLOCATED, &srq);
> -       if (err)
> -               return err;
> +       srq = srq_res_start_move_to(dev, slave, srqn, RES_SRQ_ALLOCATED);
> +       if (IS_ERR(srq))
> +               return PTR_ERR(srq);
>         err = mlx4_DMA_wrapper(dev, slave, vhcr, inbox, outbox, cmd);
>         if (err)
>                 goto ex_abort;
> --
> 1.7.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH 2/2] Re: net, mellanox mlx4 Fix compile warnings
From: Prarit Bhargava @ 2013-09-17 19:13 UTC (permalink / raw)
  To: netdev; +Cc: Prarit Bhargava, dledford, amirv, davem, ogerlitz
In-Reply-To: <20130916.212559.252825159014181091.davem@davemloft.net>

Fix unitialized variable warnings.

drivers/net/ethernet/mellanox/mlx4/resource_tracker.c: In function ‘mlx4_HW2SW_CQ_wrapper’:
drivers/net/ethernet/mellanox/mlx4/resource_tracker.c:2551:16: error: ‘cq’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  atomic_dec(&cq->mtt->ref_count);
                ^
drivers/net/ethernet/mellanox/mlx4/resource_tracker.c: In function ‘mlx4_HW2SW_SRQ_wrapper’:
drivers/net/ethernet/mellanox/mlx4/resource_tracker.c:2734:17: error: ‘srq’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  atomic_dec(&srq->mtt->ref_count);

[v2]: davem suggested making cq_res_start_move_to() return 'cq' as an error
pointer instead of setting 'cq' by reference.  I also did the same for
srq.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: dledford@redhat.com
Cc: amirv@mellanox.com
Cc: davem@davemloft.net
Cc: ogerlitz@mellanox.com
---
 .../net/ethernet/mellanox/mlx4/resource_tracker.c  |   46 ++++++++++----------
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
index dd68763..343206b 100644
--- a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
+++ b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
@@ -1073,8 +1073,9 @@ static int eq_res_start_move_to(struct mlx4_dev *dev, int slave, int index,
 	return err;
 }
 
-static int cq_res_start_move_to(struct mlx4_dev *dev, int slave, int cqn,
-				enum res_cq_states state, struct res_cq **cq)
+static struct res_cq *cq_res_start_move_to(struct mlx4_dev *dev,
+						  int slave, int cqn,
+						  enum res_cq_states state)
 {
 	struct mlx4_priv *priv = mlx4_priv(dev);
 	struct mlx4_resource_tracker *tracker = &priv->mfunc.master.res_tracker;
@@ -1117,18 +1118,19 @@ static int cq_res_start_move_to(struct mlx4_dev *dev, int slave, int cqn,
 			r->com.from_state = r->com.state;
 			r->com.to_state = state;
 			r->com.state = RES_CQ_BUSY;
-			if (cq)
-				*cq = r;
+		} else {
+			r = ERR_PTR(err);
 		}
 	}
 
 	spin_unlock_irq(mlx4_tlock(dev));
 
-	return err;
+	return r;
 }
 
-static int srq_res_start_move_to(struct mlx4_dev *dev, int slave, int index,
-				 enum res_cq_states state, struct res_srq **srq)
+static struct res_srq *srq_res_start_move_to(struct mlx4_dev *dev, int slave,
+					     int index,
+					     enum res_cq_states state)
 {
 	struct mlx4_priv *priv = mlx4_priv(dev);
 	struct mlx4_resource_tracker *tracker = &priv->mfunc.master.res_tracker;
@@ -1167,14 +1169,14 @@ static int srq_res_start_move_to(struct mlx4_dev *dev, int slave, int index,
 			r->com.from_state = r->com.state;
 			r->com.to_state = state;
 			r->com.state = RES_SRQ_BUSY;
-			if (srq)
-				*srq = r;
+		} else {
+			r = ERR_PTR(err);
 		}
 	}
 
 	spin_unlock_irq(mlx4_tlock(dev));
 
-	return err;
+	return r;
 }
 
 static void res_abort_move(struct mlx4_dev *dev, int slave,
@@ -2530,9 +2532,9 @@ int mlx4_SW2HW_CQ_wrapper(struct mlx4_dev *dev, int slave,
 	struct res_cq *cq;
 	struct res_mtt *mtt;
 
-	err = cq_res_start_move_to(dev, slave, cqn, RES_CQ_HW, &cq);
-	if (err)
-		return err;
+	cq = cq_res_start_move_to(dev, slave, cqn, RES_CQ_HW);
+	if (IS_ERR(cq))
+		return PTR_ERR(cq);
 	err = get_res(dev, slave, mtt_base, RES_MTT, &mtt);
 	if (err)
 		goto out_move;
@@ -2565,9 +2567,9 @@ int mlx4_HW2SW_CQ_wrapper(struct mlx4_dev *dev, int slave,
 	int cqn = vhcr->in_modifier;
 	struct res_cq *cq;
 
-	err = cq_res_start_move_to(dev, slave, cqn, RES_CQ_ALLOCATED, &cq);
-	if (err)
-		return err;
+	cq = cq_res_start_move_to(dev, slave, cqn, RES_CQ_ALLOCATED);
+	if (IS_ERR(cq))
+		return PTR_ERR(cq);
 	err = mlx4_DMA_wrapper(dev, slave, vhcr, inbox, outbox, cmd);
 	if (err)
 		goto out_move;
@@ -2709,9 +2711,9 @@ int mlx4_SW2HW_SRQ_wrapper(struct mlx4_dev *dev, int slave,
 	if (srqn != (be32_to_cpu(srqc->state_logsize_srqn) & 0xffffff))
 		return -EINVAL;
 
-	err = srq_res_start_move_to(dev, slave, srqn, RES_SRQ_HW, &srq);
-	if (err)
-		return err;
+	srq = srq_res_start_move_to(dev, slave, srqn, RES_SRQ_ALLOCATED);
+	if (IS_ERR(srq))
+		return PTR_ERR(srq);
 	err = get_res(dev, slave, mtt_base, RES_MTT, &mtt);
 	if (err)
 		goto ex_abort;
@@ -2748,9 +2750,9 @@ int mlx4_HW2SW_SRQ_wrapper(struct mlx4_dev *dev, int slave,
 	int srqn = vhcr->in_modifier;
 	struct res_srq *srq;
 
-	err = srq_res_start_move_to(dev, slave, srqn, RES_SRQ_ALLOCATED, &srq);
-	if (err)
-		return err;
+	srq = srq_res_start_move_to(dev, slave, srqn, RES_SRQ_ALLOCATED);
+	if (IS_ERR(srq))
+		return PTR_ERR(srq);
 	err = mlx4_DMA_wrapper(dev, slave, vhcr, inbox, outbox, cmd);
 	if (err)
 		goto ex_abort;
-- 
1.7.9.3

^ permalink raw reply related

* [PATCH 1/2] Re: net, vxlan Fix compile warning
From: Prarit Bhargava @ 2013-09-17 19:12 UTC (permalink / raw)
  To: netdev; +Cc: Prarit Bhargava, jpirko, davem, stephen
In-Reply-To: <20130916.212358.1356743853860170770.davem@davemloft.net>

Fix a unintialized variable warning.

drivers/net/vxlan.c: In function ‘vxlan_sock_add’:
drivers/net/vxlan.c:2240:11: error: ‘sock’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  vs->sock = sock;
           ^
drivers/net/vxlan.c:2217:17: note: ‘sock’ was declared here
  struct socket *sock;
                 ^
[v2]: davem suggested resolving this by making create_v{4,6}_sock() return an
err pointer.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: jpirko@redhat.com
Cc: davem@davemloft.net
Cc: stephen@networkplumber.org
---
 drivers/net/vxlan.c |   26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index bf64b41..6ec6aa4 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -2184,7 +2184,7 @@ static void vxlan_del_work(struct work_struct *work)
  * could be used for both IPv4 and IPv6 communications, but
  * users may set bindv6only=1.
  */
-static int create_v6_sock(struct net *net, __be16 port, struct socket **psock)
+static struct socket *create_v6_sock(struct net *net, __be16 port)
 {
 	struct sock *sk;
 	struct socket *sock;
@@ -2197,7 +2197,7 @@ static int create_v6_sock(struct net *net, __be16 port, struct socket **psock)
 	rc = sock_create_kern(AF_INET6, SOCK_DGRAM, IPPROTO_UDP, &sock);
 	if (rc < 0) {
 		pr_debug("UDPv6 socket create failed\n");
-		return rc;
+		return ERR_PTR(rc);
 	}
 
 	/* Put in proper namespace */
@@ -2212,28 +2212,27 @@ static int create_v6_sock(struct net *net, __be16 port, struct socket **psock)
 		pr_debug("bind for UDPv6 socket %pI6:%u (%d)\n",
 			 &vxlan_addr.sin6_addr, ntohs(vxlan_addr.sin6_port), rc);
 		sk_release_kernel(sk);
-		return rc;
+		return ERR_PTR(rc);
 	}
 	/* At this point, IPv6 module should have been loaded in
 	 * sock_create_kern().
 	 */
 	BUG_ON(!ipv6_stub);
 
-	*psock = sock;
 	/* Disable multicast loopback */
 	inet_sk(sk)->mc_loop = 0;
-	return 0;
+	return sock;
 }
 
 #else
 
-static int create_v6_sock(struct net *net, __be16 port, struct socket **psock)
+static struct socket *create_v6_sock(struct net *net, __be16 port)
 {
-		return -EPFNOSUPPORT;
+		return ERR_PTR(-EPFNOSUPPORT);
 }
 #endif
 
-static int create_v4_sock(struct net *net, __be16 port, struct socket **psock)
+static struct socket *create_v4_sock(struct net *net, __be16 port)
 {
 	struct sock *sk;
 	struct socket *sock;
@@ -2248,7 +2247,7 @@ static int create_v4_sock(struct net *net, __be16 port, struct socket **psock)
 	rc = sock_create_kern(AF_INET, SOCK_DGRAM, IPPROTO_UDP, &sock);
 	if (rc < 0) {
 		pr_debug("UDP socket create failed\n");
-		return rc;
+		return ERR_PTR(rc);
 	}
 
 	/* Put in proper namespace */
@@ -2261,13 +2260,12 @@ static int create_v4_sock(struct net *net, __be16 port, struct socket **psock)
 		pr_debug("bind for UDP socket %pI4:%u (%d)\n",
 			 &vxlan_addr.sin_addr, ntohs(vxlan_addr.sin_port), rc);
 		sk_release_kernel(sk);
-		return rc;
+		return ERR_PTR(rc);
 	}
 
-	*psock = sock;
 	/* Disable multicast loopback */
 	inet_sk(sk)->mc_loop = 0;
-	return 0;
+	return sock;
 }
 
 /* Create new listen socket if needed */
@@ -2291,9 +2289,9 @@ static struct vxlan_sock *vxlan_socket_create(struct net *net, __be16 port,
 	INIT_WORK(&vs->del_work, vxlan_del_work);
 
 	if (ipv6)
-		rc = create_v6_sock(net, port, &sock);
+		sock = create_v6_sock(net, port);
 	else
-		rc = create_v4_sock(net, port, &sock);
+		sock = create_v4_sock(net, port);
 	if (rc < 0) {
 		kfree(vs);
 		return ERR_PTR(rc);
-- 
1.7.9.3

^ permalink raw reply related

* [PATCH] vxlan: Avoid creating fdb entry with NULL destination
From: Sridhar Samudrala @ 2013-09-17 19:12 UTC (permalink / raw)
  To: davem; +Cc: netdev, stephen, mike.rapoport

Commit afbd8bae9c798c5cdbe4439d3a50536b5438247c 
   vxlan: add implicit fdb entry for default destination
creates an implicit fdb entry for default destination. This results 
in an invalid fdb entry if default destination is not specified.
For ex: 
  ip link add vxlan1 type vxlan id 100
creates the following fdb entry
  00:00:00:00:00:00 dev vxlan1 dst 0.0.0.0 self permanent

This patch fixes this issue by creating an fdb entry only if a
valid default destination is specified.

Signed-off-by: Sridhar Samudrala <sri@us.ibm.com>
---

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index bf64b41..ac25c2d 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -2492,15 +2492,19 @@ static int vxlan_newlink(struct net *net, struct net_device *dev,
 
 	SET_ETHTOOL_OPS(dev, &vxlan_ethtool_ops);
 
-	/* create an fdb entry for default destination */
-	err = vxlan_fdb_create(vxlan, all_zeros_mac,
-			       &vxlan->default_dst.remote_ip,
-			       NUD_REACHABLE|NUD_PERMANENT,
-			       NLM_F_EXCL|NLM_F_CREATE,
-			       vxlan->dst_port, vxlan->default_dst.remote_vni,
-			       vxlan->default_dst.remote_ifindex, NTF_SELF);
-	if (err)
-		return err;
+	/* create an fdb entry for a valid default destination */
+	if (!vxlan_addr_any(&vxlan->default_dst.remote_ip)) {
+		err = vxlan_fdb_create(vxlan, all_zeros_mac,
+				       &vxlan->default_dst.remote_ip,
+				       NUD_REACHABLE|NUD_PERMANENT,
+				       NLM_F_EXCL|NLM_F_CREATE,
+				       vxlan->dst_port,
+				       vxlan->default_dst.remote_vni,
+				       vxlan->default_dst.remote_ifindex,
+				       NTF_SELF);
+		if (err)
+			return err;
+	}
 
 	err = register_netdevice(dev);
 	if (err) {

^ permalink raw reply related

* Re: [PATCH v2.39 7/7] datapath: Add basic MPLS support to kernel
From: Pravin Shelar @ 2013-09-17 18:38 UTC (permalink / raw)
  To: Simon Horman
  Cc: dev@openvswitch.org, netdev, Ravi K, Isaku Yamahata, Jesse Gross,
	Joe Stringer
In-Reply-To: <1378711207-29890-8-git-send-email-horms@verge.net.au>

On Mon, Sep 9, 2013 at 12:20 AM, Simon Horman <horms@verge.net.au> wrote:
> Allow datapath to recognize and extract MPLS labels into flow keys
> and execute actions which push, pop, and set labels on packets.
>
> Based heavily on work by Leo Alterman, Ravi K, Isaku Yamahata and Joe Stringer.
>
> Cc: Ravi K <rkerur@gmail.com>
> Cc: Leo Alterman <lalterman@nicira.com>
> Cc: Isaku Yamahata <yamahata@valinux.co.jp>
> Cc: Joe Stringer <joe@wand.net.nz>
> Signed-off-by: Simon Horman <horms@verge.net.au>
>
> ---
....
> diff --git a/datapath/datapath.h b/datapath/datapath.h
> index 5d50dd4..babae3b 100644
> --- a/datapath/datapath.h
> +++ b/datapath/datapath.h
> @@ -36,6 +36,10 @@
>
>  #define SAMPLE_ACTION_DEPTH 3
>
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,11,0)
> +#define HAVE_INNER_PROTOCOL
> +#endif
> +
>  /**
>   * struct dp_stats_percpu - per-cpu packet processing statistics for a given
>   * datapath.
> @@ -93,11 +97,16 @@ struct datapath {
>   * @pkt_key: The flow information extracted from the packet.  Must be nonnull.
>   * @tun_key: Key for the tunnel that encapsulated this packet. NULL if the
>   * packet is not being tunneled.
> + * @inner_protocol: Provides a substitute for the skb->inner_protocol field on
> + * kernels before 3.11.
>   */
>  struct ovs_skb_cb {
>         struct sw_flow          *flow;
>         struct sw_flow_key      *pkt_key;
>         struct ovs_key_ipv4_tunnel  *tun_key;
> +#ifndef HAVE_INNER_PROTOCOL
> +       __be16                  inner_protocol;
> +#endif
>  };
>  #define OVS_CB(skb) ((struct ovs_skb_cb *)(skb)->cb)
>
Can you move this to compat struct ovs_gso_cb {}

....
>
> +struct sk_buff *rpl___skb_gso_segment(struct sk_buff *skb,
> +                                     netdev_features_t features,
> +                                     bool tx_path)
> +{
> +       struct sk_buff *skb_gso;
> +       __be16 type = skb->protocol;
> +
> +       skb->protocol = skb_network_protocol(skb);
> +
> +       /* this hack needed to get regular skb_gso_segment() */
> +#ifdef HAVE___SKB_GSO_SEGMENT
> +#undef __skb_gso_segment
> +       skb_gso = __skb_gso_segment(skb, features, tx_path);
> +#else
> +#undef skb_gso_segment
> +       skb_gso = skb_gso_segment(skb, features);
> +#endif
> +
> +       if (!skb_gso || IS_ERR(skb_gso))
> +           return skb_gso;
> +
> +       skb = skb_gso;
> +       while (skb) {
> +               skb->protocol = type;
> +               skb = skb->next;
> +       }
> +

Protocol set is required if there is MPLS header, which is rare case.
So I think we can skip this loop if there is no mpls.

> +       return skb_gso;
> +}
> +
> +struct sk_buff *rpl_skb_gso_segment(struct sk_buff *skb,
.....
> +
> +       if (vlan_tx_tag_present(skb) && !dev_supports_vlan_tx(skb->dev))
> +               vlan = true;
> +
> +       if (vlan || mpls) {
> +               netdev_features_t features;
>
>                 features = netif_skb_features(skb);
>
> @@ -296,6 +309,20 @@ static int netdev_send(struct vport *vport, struct sk_buff *skb)
>                         features &= ~(NETIF_F_TSO | NETIF_F_TSO6 |
>                                       NETIF_F_UFO | NETIF_F_FSO);
>
> +               /* As of v3.11 the kernel provides an mpls_features field in
> +                * struct net_device which allows devices to advertise which
> +                * features its supports for MPLS. This value defaults to
> +                * NETIF_F_SG and as of v3.11.
> +                *
> +                * This compatibility code is intended for kernels older
> +                * than v3.11 that do not support MPLS GSO and thus do not
> +                * provide mpls_features. Thus this code uses NETIF_F_SG
> +                * directly in place of mpls_features.
> +                */
> +
> +               if (mpls)
> +                       features &= NETIF_F_SG;
> +
>                 if (netif_needs_gso(skb, features)) {
>                         struct sk_buff *nskb;
>
> @@ -319,10 +346,12 @@ static int netdev_send(struct vport *vport, struct sk_buff *skb)
>                                 nskb = skb->next;
>                                 skb->next = NULL;
>
> -                               skb = __vlan_put_tag(skb, skb->vlan_proto, vlan_tx_tag_get(skb));
> +                               if (vlan)
> +                                       skb = __vlan_put_tag(skb, skb->vlan_proto, vlan_tx_tag_get(skb));
>                                 if (likely(skb)) {
>                                         len += skb->len;
> -                                       vlan_set_tci(skb, 0);
> +                                       if (vlan)
> +                                               vlan_set_tci(skb, 0);
>                                         dev_queue_xmit(skb);
>                                 }
>
> @@ -333,10 +362,12 @@ static int netdev_send(struct vport *vport, struct sk_buff *skb)
>                 }
>
>  tag:
> -               skb = __vlan_put_tag(skb, skb->vlan_proto, vlan_tx_tag_get(skb));
> -               if (unlikely(!skb))
> -                       return 0;
> -               vlan_set_tci(skb, 0);
> +               if (vlan) {
> +                       skb = __vlan_put_tag(skb, skb->vlan_proto, vlan_tx_tag_get(skb));
> +                       if (unlikely(!skb))
> +                               return 0;
> +                       vlan_set_tci(skb, 0);
> +               }
>         }
>
I think we can simplify code by pushing vlan and then segmenting skb,
the way we do it for MPLS.

^ permalink raw reply

* Re: [PATCH] vhost/scsi: use vmalloc for order-10 allocation
From: Sergei Shtylyov @ 2013-09-17 18:14 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: kvm, netdev, linux-kernel, virtualization, Dan Aloni
In-Reply-To: <1379401998-5131-1-git-send-email-mst@redhat.com>

Hello.

On 09/17/2013 11:21 AM, Michael S. Tsirkin wrote:

> As vhost scsi device struct is large, if the device is
> created on a busy system, kzalloc() might fail, so this patch does a
> fallback to vzalloc().

> As vmalloc() adds overhead on data-path, add __GFP_REPEAT
> to kzalloc() flags to do this fallback only when really needed.

> Reported-by: Dan Aloni <alonid@stratoscale.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---

> I put this on my vhost fixes branch, intend to merge for 3.12.
> Dan, could you please confirm this works for you?

>   drivers/vhost/scsi.c | 41 +++++++++++++++++++++++++++--------------
>   1 file changed, 27 insertions(+), 14 deletions(-)

> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> index 4b79a1f..2c30bb0 100644
> --- a/drivers/vhost/scsi.c
> +++ b/drivers/vhost/scsi.c
> @@ -1373,21 +1373,30 @@ static int vhost_scsi_set_features(struct vhost_scsi *vs, u64 features)
>   	return 0;
>   }
>
> +static void vhost_scsi_free(struct vhost_scsi *vs)
> +{
> +        if (is_vmalloc_addr(vs))
> +                vfree(vs);
> +        else
> +                kfree(vs);

    Indent with the tabs ISO spaces, please.

> +}
> +
>   static int vhost_scsi_open(struct inode *inode, struct file *f)
>   {
>   	struct vhost_scsi *vs;
>   	struct vhost_virtqueue **vqs;
> -	int r, i;
> +	int r = -ENOMEM, i;
>
> -	vs = kzalloc(sizeof(*vs), GFP_KERNEL);
> -	if (!vs)
> -		return -ENOMEM;
> +        vs = kzalloc(sizeof(*vs), GFP_KERNEL | __GFP_NOWARN | __GFP_REPEAT);

    Indent here with a tab, please.

> +	if (!vs) {
> +		vs = vzalloc(sizeof(*vs));
> +		if (!vs)
> +			goto err_vs;
> +	}

WBR, Sergei

^ permalink raw reply

* Re: [PATCH -net] netpoll: fix NULL pointer dereference in netpoll_cleanup
From: Nikolay Aleksandrov @ 2013-09-17 18:06 UTC (permalink / raw)
  To: netdev; +Cc: davem
In-Reply-To: <52386911.2010504@redhat.com>

On 09/17/2013 04:37 PM, Nikolay Aleksandrov wrote:
> commit 7a163bfb7ce50895bbe67300ea610d31b9c09230
> ("netconsole: avoid a crash with multiple sysfs writers")
I feel like I didn't explain this one well. The above commit actually
tries to fix the same issue AFAICT, and it can be reverted if/once my fix
is accepted, but I think to remove only the locking in the netconsole
netdev notifier to avoid the deadlock because the mutex lock is useful for
fixing a third bug in netconsole, that I intend to take care of once this
is sorted out.
So basically my next fix that takes care of the deadlock is dependent on
this patch, and I'll wait to see the feedback, if it gets accepted I'll
post the follow-up that takes care of the deadlock.

Cheers,
 Nik

^ permalink raw reply

* Re: [CFT][PATCH] net: Delay default_device_exit_batch until no devices are unregistering
From: Francesco Ruggeri @ 2013-09-17 17:14 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: David S. Miller, Eric Dumazet, Jiri Pirko, Alexander Duyck,
	Cong Wang, netdev
In-Reply-To: <87mwnb949b.fsf@xmission.com>

>
> If you could test this patch perhaps refine it I think we are almost at
> a final point of fixing this.
>
I will.

> Just to be clear my reason for prefering this approach is that because
> it adds no extra wait points (we already wait for the rtnl_lock), the
> logic is unconditional and explicit and not hidden in the loopback
> device's reference count.  Which should allow anyone reading the code
> to discover and understand this guarantee.  Although a big fat comment
> in default_device_exit_batch that we are guaranteeing we don't allow
> the network namespace to exit while there are still network devices in
> it (or something to that effect) is probably appropriate.

I agree, this approach is cleaner than overloading loopback_dev's
refcount as in my original patch.
I will let you know how my tests go over the next few days.

Thanks,
Francesco


>
> Eric
>

^ permalink raw reply

* RE: [PATCH v4] Don't destroy the netdev until the vif is shut down
From: Paul Durrant @ 2013-09-17 16:45 UTC (permalink / raw)
  To: Paul Durrant, xen-devel@lists.xen.org, netdev@vger.kernel.org
  Cc: David Vrabel, Wei Liu, Ian Campbell
In-Reply-To: <1379432723-24384-1-git-send-email-paul.durrant@citrix.com>

> -----Original Message-----
> From: Paul Durrant [mailto:paul.durrant@citrix.com]
> Sent: 17 September 2013 16:45
> To: xen-devel@lists.xen.org; netdev@vger.kernel.org
> Cc: Paul Durrant; David Vrabel; Wei Liu; Ian Campbell
> Subject: [PATCH v4] Don't destroy the netdev until the vif is shut down
> 

Apologies for the loss of the net-next prefix here. I'll re-send with the prefix.

  Paul

> Without this patch, if a frontend cycles through states Closing
> and Closed (which Windows frontends need to do) then the netdev
> will be destroyed and requires re-invocation of hotplug scripts
> to restore state before the frontend can move to Connected. Thus
> when udev is not in use the backend gets stuck in InitWait.
> 
> With this patch, the netdev is left alone whilst the backend is
> still online and is only de-registered and freed just prior to
> destroying the vif (which is also nicely symmetrical with the
> netdev allocation and registration being done during probe) so
> no re-invocation of hotplug scripts is required.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Cc: David Vrabel <david.vrabel@citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> ---
> v2:
>  - Modify netback_remove() - bug only seemed to manifest with linux guest
> 
> v3:
>  - Move __module_get() and module_put() calls
> 
> v4:
>  - Clear tx_irq in xenvif_disconnect() to make sure that a subsequent
>    call to xenvif_connect() doesn't get nop-ed.
> 
>  drivers/net/xen-netback/common.h    |    1 +
>  drivers/net/xen-netback/interface.c |   26 ++++++++++----------------
>  drivers/net/xen-netback/xenbus.c    |   17 ++++++++++++-----
>  3 files changed, 23 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-
> netback/common.h
> index a197743..5715318 100644
> --- a/drivers/net/xen-netback/common.h
> +++ b/drivers/net/xen-netback/common.h
> @@ -184,6 +184,7 @@ int xenvif_connect(struct xenvif *vif, unsigned long
> tx_ring_ref,
>  		   unsigned long rx_ring_ref, unsigned int tx_evtchn,
>  		   unsigned int rx_evtchn);
>  void xenvif_disconnect(struct xenvif *vif);
> +void xenvif_free(struct xenvif *vif);
> 
>  int xenvif_xenbus_init(void);
>  void xenvif_xenbus_fini(void);
> diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-
> netback/interface.c
> index 625c6f4..0465e0f 100644
> --- a/drivers/net/xen-netback/interface.c
> +++ b/drivers/net/xen-netback/interface.c
> @@ -353,6 +353,9 @@ struct xenvif *xenvif_alloc(struct device *parent,
> domid_t domid,
>  	}
> 
>  	netdev_dbg(dev, "Successfully created xenvif\n");
> +
> +	__module_get(THIS_MODULE);
> +
>  	return vif;
>  }
> 
> @@ -366,8 +369,6 @@ int xenvif_connect(struct xenvif *vif, unsigned long
> tx_ring_ref,
>  	if (vif->tx_irq)
>  		return 0;
> 
> -	__module_get(THIS_MODULE);
> -
>  	err = xenvif_map_frontend_rings(vif, tx_ring_ref, rx_ring_ref);
>  	if (err < 0)
>  		goto err;
> @@ -452,12 +453,6 @@ void xenvif_carrier_off(struct xenvif *vif)
> 
>  void xenvif_disconnect(struct xenvif *vif)
>  {
> -	/* Disconnect funtion might get called by generic framework
> -	 * even before vif connects, so we need to check if we really
> -	 * need to do a module_put.
> -	 */
> -	int need_module_put = 0;
> -
>  	if (netif_carrier_ok(vif->dev))
>  		xenvif_carrier_off(vif);
> 
> @@ -468,23 +463,22 @@ void xenvif_disconnect(struct xenvif *vif)
>  			unbind_from_irqhandler(vif->tx_irq, vif);
>  			unbind_from_irqhandler(vif->rx_irq, vif);
>  		}
> -		/* vif->irq is valid, we had a module_get in
> -		 * xenvif_connect.
> -		 */
> -		need_module_put = 1;
> +		vif->tx_irq = 0;
>  	}
> 
>  	if (vif->task)
>  		kthread_stop(vif->task);
> 
> +	xenvif_unmap_frontend_rings(vif);
> +}
> +
> +void xenvif_free(struct xenvif *vif)
> +{
>  	netif_napi_del(&vif->napi);
> 
>  	unregister_netdev(vif->dev);
> 
> -	xenvif_unmap_frontend_rings(vif);
> -
>  	free_netdev(vif->dev);
> 
> -	if (need_module_put)
> -		module_put(THIS_MODULE);
> +	module_put(THIS_MODULE);
>  }
> diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-
> netback/xenbus.c
> index 1fe48fe3..a53782e 100644
> --- a/drivers/net/xen-netback/xenbus.c
> +++ b/drivers/net/xen-netback/xenbus.c
> @@ -42,7 +42,7 @@ static int netback_remove(struct xenbus_device *dev)
>  	if (be->vif) {
>  		kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE);
>  		xenbus_rm(XBT_NIL, dev->nodename, "hotplug-status");
> -		xenvif_disconnect(be->vif);
> +		xenvif_free(be->vif);
>  		be->vif = NULL;
>  	}
>  	kfree(be);
> @@ -213,9 +213,18 @@ static void disconnect_backend(struct
> xenbus_device *dev)
>  {
>  	struct backend_info *be = dev_get_drvdata(&dev->dev);
> 
> +	if (be->vif)
> +		xenvif_disconnect(be->vif);
> +}
> +
> +static void destroy_backend(struct xenbus_device *dev)
> +{
> +	struct backend_info *be = dev_get_drvdata(&dev->dev);
> +
>  	if (be->vif) {
> +		kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE);
>  		xenbus_rm(XBT_NIL, dev->nodename, "hotplug-status");
> -		xenvif_disconnect(be->vif);
> +		xenvif_free(be->vif);
>  		be->vif = NULL;
>  	}
>  }
> @@ -246,14 +255,11 @@ static void frontend_changed(struct xenbus_device
> *dev,
>  	case XenbusStateConnected:
>  		if (dev->state == XenbusStateConnected)
>  			break;
> -		backend_create_xenvif(be);
>  		if (be->vif)
>  			connect(be);
>  		break;
> 
>  	case XenbusStateClosing:
> -		if (be->vif)
> -			kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE);
>  		disconnect_backend(dev);
>  		xenbus_switch_state(dev, XenbusStateClosing);
>  		break;
> @@ -262,6 +268,7 @@ static void frontend_changed(struct xenbus_device
> *dev,
>  		xenbus_switch_state(dev, XenbusStateClosed);
>  		if (xenbus_dev_is_online(dev))
>  			break;
> +		destroy_backend(dev);
>  		/* fall through if not online */
>  	case XenbusStateUnknown:
>  		device_unregister(&dev->dev);
> --
> 1.7.10.4

^ permalink raw reply

* [PATCH net-next v4] Don't destroy the netdev until the vif is shut down
From: Paul Durrant @ 2013-09-17 16:46 UTC (permalink / raw)
  To: xen-devel, netdev; +Cc: Paul Durrant, David Vrabel, Wei Liu, Ian Campbell

Without this patch, if a frontend cycles through states Closing
and Closed (which Windows frontends need to do) then the netdev
will be destroyed and requires re-invocation of hotplug scripts
to restore state before the frontend can move to Connected. Thus
when udev is not in use the backend gets stuck in InitWait.

With this patch, the netdev is left alone whilst the backend is
still online and is only de-registered and freed just prior to
destroying the vif (which is also nicely symmetrical with the
netdev allocation and registration being done during probe) so
no re-invocation of hotplug scripts is required.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Cc: David Vrabel <david.vrabel@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
---
v2:
 - Modify netback_remove() - bug only seemed to manifest with linux guest

v3:
 - Move __module_get() and module_put() calls

v4:
 - Clear tx_irq in xenvif_disconnect() to make sure that a subsequent
   call to xenvif_connect() doesn't get nop-ed.

 drivers/net/xen-netback/common.h    |    1 +
 drivers/net/xen-netback/interface.c |   26 ++++++++++----------------
 drivers/net/xen-netback/xenbus.c    |   17 ++++++++++++-----
 3 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index a197743..5715318 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -184,6 +184,7 @@ int xenvif_connect(struct xenvif *vif, unsigned long tx_ring_ref,
 		   unsigned long rx_ring_ref, unsigned int tx_evtchn,
 		   unsigned int rx_evtchn);
 void xenvif_disconnect(struct xenvif *vif);
+void xenvif_free(struct xenvif *vif);
 
 int xenvif_xenbus_init(void);
 void xenvif_xenbus_fini(void);
diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
index 625c6f4..0465e0f 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -353,6 +353,9 @@ struct xenvif *xenvif_alloc(struct device *parent, domid_t domid,
 	}
 
 	netdev_dbg(dev, "Successfully created xenvif\n");
+
+	__module_get(THIS_MODULE);
+
 	return vif;
 }
 
@@ -366,8 +369,6 @@ int xenvif_connect(struct xenvif *vif, unsigned long tx_ring_ref,
 	if (vif->tx_irq)
 		return 0;
 
-	__module_get(THIS_MODULE);
-
 	err = xenvif_map_frontend_rings(vif, tx_ring_ref, rx_ring_ref);
 	if (err < 0)
 		goto err;
@@ -452,12 +453,6 @@ void xenvif_carrier_off(struct xenvif *vif)
 
 void xenvif_disconnect(struct xenvif *vif)
 {
-	/* Disconnect funtion might get called by generic framework
-	 * even before vif connects, so we need to check if we really
-	 * need to do a module_put.
-	 */
-	int need_module_put = 0;
-
 	if (netif_carrier_ok(vif->dev))
 		xenvif_carrier_off(vif);
 
@@ -468,23 +463,22 @@ void xenvif_disconnect(struct xenvif *vif)
 			unbind_from_irqhandler(vif->tx_irq, vif);
 			unbind_from_irqhandler(vif->rx_irq, vif);
 		}
-		/* vif->irq is valid, we had a module_get in
-		 * xenvif_connect.
-		 */
-		need_module_put = 1;
+		vif->tx_irq = 0;
 	}
 
 	if (vif->task)
 		kthread_stop(vif->task);
 
+	xenvif_unmap_frontend_rings(vif);
+}
+
+void xenvif_free(struct xenvif *vif)
+{
 	netif_napi_del(&vif->napi);
 
 	unregister_netdev(vif->dev);
 
-	xenvif_unmap_frontend_rings(vif);
-
 	free_netdev(vif->dev);
 
-	if (need_module_put)
-		module_put(THIS_MODULE);
+	module_put(THIS_MODULE);
 }
diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index 1fe48fe3..a53782e 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -42,7 +42,7 @@ static int netback_remove(struct xenbus_device *dev)
 	if (be->vif) {
 		kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE);
 		xenbus_rm(XBT_NIL, dev->nodename, "hotplug-status");
-		xenvif_disconnect(be->vif);
+		xenvif_free(be->vif);
 		be->vif = NULL;
 	}
 	kfree(be);
@@ -213,9 +213,18 @@ static void disconnect_backend(struct xenbus_device *dev)
 {
 	struct backend_info *be = dev_get_drvdata(&dev->dev);
 
+	if (be->vif)
+		xenvif_disconnect(be->vif);
+}
+
+static void destroy_backend(struct xenbus_device *dev)
+{
+	struct backend_info *be = dev_get_drvdata(&dev->dev);
+
 	if (be->vif) {
+		kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE);
 		xenbus_rm(XBT_NIL, dev->nodename, "hotplug-status");
-		xenvif_disconnect(be->vif);
+		xenvif_free(be->vif);
 		be->vif = NULL;
 	}
 }
@@ -246,14 +255,11 @@ static void frontend_changed(struct xenbus_device *dev,
 	case XenbusStateConnected:
 		if (dev->state == XenbusStateConnected)
 			break;
-		backend_create_xenvif(be);
 		if (be->vif)
 			connect(be);
 		break;
 
 	case XenbusStateClosing:
-		if (be->vif)
-			kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE);
 		disconnect_backend(dev);
 		xenbus_switch_state(dev, XenbusStateClosing);
 		break;
@@ -262,6 +268,7 @@ static void frontend_changed(struct xenbus_device *dev,
 		xenbus_switch_state(dev, XenbusStateClosed);
 		if (xenbus_dev_is_online(dev))
 			break;
+		destroy_backend(dev);
 		/* fall through if not online */
 	case XenbusStateUnknown:
 		device_unregister(&dev->dev);
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCHv2 net] xen-netback: count number required slots for an skb more carefully
From: David Vrabel @ 2013-09-17 16:41 UTC (permalink / raw)
  To: Ian Campbell
  Cc: David Miller, xen-devel, konrad.wilk, boris.ostrovsky, netdev
In-Reply-To: <1379426727.11304.103.camel@hastur.hellion.org.uk>

On 17/09/13 15:05, Ian Campbell wrote:
> On Thu, 2013-09-12 at 23:23 -0400, David Miller wrote:
>> I assume you want this queued up for -stable,
> 
> I think so, David V -- do you know how far back this goes?

Stable please, but only back to 3.10.

I suspect (but never had time to confirm) that this fixes a regression
introduced by "xen/netback: Calculate the number of SKB slots required
correctly" e26b203e which was added in 3.6.

>>  and can you check if
>> there is any non-trivial backporting for earlier kernels?
> 
> Another one I hope David can answer...

Backport to 3.10 "just works".

David

^ 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