Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next] ipv6 xfrm: Use ipv6_addr_hash() in xfrm6_tunnel_spi_hash_byaddr().
From: YOSHIFUJI Hideaki @ 2013-01-13  8:32 UTC (permalink / raw)
  To: davem, netdev; +Cc: yoshfuji

Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 net/ipv6/xfrm6_tunnel.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv6/xfrm6_tunnel.c b/net/ipv6/xfrm6_tunnel.c
index ee5a706..babd167 100644
--- a/net/ipv6/xfrm6_tunnel.c
+++ b/net/ipv6/xfrm6_tunnel.c
@@ -72,7 +72,7 @@ static inline unsigned int xfrm6_tunnel_spi_hash_byaddr(const xfrm_address_t *ad
 {
 	unsigned int h;
 
-	h = (__force u32)(addr->a6[0] ^ addr->a6[1] ^ addr->a6[2] ^ addr->a6[3]);
+	h = ipv6_addr_hash((const struct in6_addr *)addr);
 	h ^= h >> 16;
 	h ^= h >> 8;
 	h &= XFRM6_TUNNEL_SPI_BYADDR_HSIZE - 1;
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH net-next] ipv6: Use ipv6_get_dsfield() instead of ipv6_tclass().
From: YOSHIFUJI Hideaki @ 2013-01-13  8:32 UTC (permalink / raw)
  To: davem, netdev; +Cc: yoshfuji

Commit 7a3198a8 ("ipv6: helper function to get tclass") introduced
ipv6_tclass(), but similar function is already available as
ipv6_get_dsfield().

We might be able to call ipv6_tclass() from ipv6_get_dsfield(),
but it is confusing to have two versions.

Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 include/linux/ipv6.h |    5 -----
 net/ipv6/datagram.c  |    3 ++-
 net/ipv6/tcp_ipv6.c  |    6 +++---
 3 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index faed1e3..304a9f4 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -77,11 +77,6 @@ static inline struct ipv6hdr *ipipv6_hdr(const struct sk_buff *skb)
 	return (struct ipv6hdr *)skb_transport_header(skb);
 }
 
-static inline __u8 ipv6_tclass(const struct ipv6hdr *iph)
-{
-	return (ntohl(*(__be32 *)iph) >> 20) & 0xff;
-}
-
 /* 
    This structure contains results of exthdrs parsing
    as offsets from skb->nh.
diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
index 56b692b..633ac55 100644
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -30,6 +30,7 @@
 #include <net/transp_v6.h>
 #include <net/ip6_route.h>
 #include <net/tcp_states.h>
+#include <net/dsfield.h>
 
 #include <linux/errqueue.h>
 #include <asm/uaccess.h>
@@ -487,7 +488,7 @@ int datagram_recv_ctl(struct sock *sk, struct msghdr *msg, struct sk_buff *skb)
 	}
 
 	if (np->rxopt.bits.rxtclass) {
-		int tclass = ipv6_tclass(ipv6_hdr(skb));
+		int tclass = ipv6_get_dsfield(ipv6_hdr(skb));
 		put_cmsg(msg, SOL_IPV6, IPV6_TCLASS, sizeof(tclass), &tclass);
 	}
 
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 3164ad2..3701c3c 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1163,7 +1163,7 @@ static struct sock * tcp_v6_syn_recv_sock(struct sock *sk, struct sk_buff *skb,
 		newnp->opt	   = NULL;
 		newnp->mcast_oif   = inet6_iif(skb);
 		newnp->mcast_hops  = ipv6_hdr(skb)->hop_limit;
-		newnp->rcv_tclass  = ipv6_tclass(ipv6_hdr(skb));
+		newnp->rcv_tclass  = ipv6_get_dsfield(ipv6_hdr(skb));
 
 		/*
 		 * No need to charge this sock to the relevant IPv6 refcnt debug socks count
@@ -1243,7 +1243,7 @@ static struct sock * tcp_v6_syn_recv_sock(struct sock *sk, struct sk_buff *skb,
 	newnp->opt	  = NULL;
 	newnp->mcast_oif  = inet6_iif(skb);
 	newnp->mcast_hops = ipv6_hdr(skb)->hop_limit;
-	newnp->rcv_tclass = ipv6_tclass(ipv6_hdr(skb));
+	newnp->rcv_tclass = ipv6_get_dsfield(ipv6_hdr(skb));
 
 	/* Clone native IPv6 options from listening socket (if any)
 
@@ -1456,7 +1456,7 @@ ipv6_pktoptions:
 		if (np->rxopt.bits.rxhlim || np->rxopt.bits.rxohlim)
 			np->mcast_hops = ipv6_hdr(opt_skb)->hop_limit;
 		if (np->rxopt.bits.rxtclass)
-			np->rcv_tclass = ipv6_tclass(ipv6_hdr(skb));
+			np->rcv_tclass = ipv6_get_dsfield(ipv6_hdr(skb));
 		if (ipv6_opt_accepted(sk, opt_skb)) {
 			skb_set_owner_r(opt_skb, sk);
 			opt_skb = xchg(&np->pktoptions, opt_skb);
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH net-next] ipv6: Use ipv6_is_mld() in ip6_mc_input().
From: YOSHIFUJI Hideaki @ 2013-01-13  8:32 UTC (permalink / raw)
  To: davem, netdev; +Cc: yoshfuji

Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 include/net/addrconf.h |    2 +-
 net/ipv6/ip6_input.c   |   21 +++------------------
 net/ipv6/mcast.c       |    4 ++--
 3 files changed, 6 insertions(+), 21 deletions(-)

diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index df4ef94..59f14ab 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -150,7 +150,7 @@ extern void addrconf_dad_failure(struct inet6_ifaddr *ifp);
 extern bool ipv6_chk_mcast_addr(struct net_device *dev,
 				const struct in6_addr *group,
 				const struct in6_addr *src_addr);
-extern bool ipv6_is_mld(struct sk_buff *skb, int nexthdr);
+extern bool ipv6_is_mld(struct sk_buff *skb, int nexthdr, int offset);
 
 extern void addrconf_prefix_rcv(struct net_device *dev,
 				u8 *opt, int len, bool sllao);
diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
index a52d864..6de509e 100644
--- a/net/ipv6/ip6_input.c
+++ b/net/ipv6/ip6_input.c
@@ -212,7 +212,7 @@ resubmit:
 			if (ipv6_addr_is_multicast(&hdr->daddr) &&
 			    !ipv6_chk_mcast_addr(skb->dev, &hdr->daddr,
 			    &hdr->saddr) &&
-			    !ipv6_is_mld(skb, nexthdr))
+			    !ipv6_is_mld(skb, nexthdr, 0))
 				goto discard;
 		}
 		if (!(ipprot->flags & INET6_PROTO_NOPOLICY) &&
@@ -283,7 +283,6 @@ int ip6_mc_input(struct sk_buff *skb)
 		if (unlikely(opt->ra)) {
 			/* Check if this is a mld message */
 			u8 *ptr = skb_network_header(skb) + opt->ra;
-			struct icmp6hdr *icmp6;
 			u8 nexthdr = hdr->nexthdr;
 			__be16 frag_off;
 			int offset;
@@ -303,24 +302,10 @@ int ip6_mc_input(struct sk_buff *skb)
 				if (offset < 0)
 					goto out;
 
-				if (nexthdr != IPPROTO_ICMPV6)
+				if (!ipv6_is_mld(skb, offset, nexthdr))
 					goto out;
 
-				if (!pskb_may_pull(skb, (skb_network_header(skb) +
-						   offset + 1 - skb->data)))
-					goto out;
-
-				icmp6 = (struct icmp6hdr *)(skb_network_header(skb) + offset);
-
-				switch (icmp6->icmp6_type) {
-				case ICMPV6_MGM_QUERY:
-				case ICMPV6_MGM_REPORT:
-				case ICMPV6_MGM_REDUCTION:
-				case ICMPV6_MLD2_REPORT:
-					deliver = true;
-					break;
-				}
-				goto out;
+				deliver = true;
 			}
 			/* unknown RA - process it normally */
 		}
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 28dfa5f..b529c27 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -937,14 +937,14 @@ int ipv6_dev_mc_dec(struct net_device *dev, const struct in6_addr *addr)
 /*
  * identify MLD packets for MLD filter exceptions
  */
-bool ipv6_is_mld(struct sk_buff *skb, int nexthdr)
+bool ipv6_is_mld(struct sk_buff *skb, int nexthdr, int offset)
 {
 	struct icmp6hdr *pic;
 
 	if (nexthdr != IPPROTO_ICMPV6)
 		return false;
 
-	if (!pskb_may_pull(skb, sizeof(struct icmp6hdr)))
+	if (!pskb_may_pull(skb, offset + sizeof(struct icmp6hdr)))
 		return false;
 
 	pic = icmp6_hdr(skb);
-- 
1.7.9.5

^ permalink raw reply related

* Re: [PATCH net-next] ipv6: Use ipv6_is_mld() in ip6_mc_input().
From: YOSHIFUJI Hideaki @ 2013-01-13  8:43 UTC (permalink / raw)
  To: davem; +Cc: YOSHIFUJI Hideaki, netdev
In-Reply-To: <50F27111.9020001@linux-ipv6.org>

YOSHIFUJI Hideaki wrote:
> Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> ---
>  include/net/addrconf.h |    2 +-
>  net/ipv6/ip6_input.c   |   21 +++------------------
>  net/ipv6/mcast.c       |    4 ++--
>  3 files changed, 6 insertions(+), 21 deletions(-)
> 
> diff --git a/include/net/addrconf.h b/include/net/addrconf.h
> index df4ef94..59f14ab 100644
> --- a/include/net/addrconf.h
> +++ b/include/net/addrconf.h
> @@ -150,7 +150,7 @@ extern void addrconf_dad_failure(struct inet6_ifaddr *ifp);
>  extern bool ipv6_chk_mcast_addr(struct net_device *dev,
>  				const struct in6_addr *group,
>  				const struct in6_addr *src_addr);
> -extern bool ipv6_is_mld(struct sk_buff *skb, int nexthdr);
> +extern bool ipv6_is_mld(struct sk_buff *skb, int nexthdr, int offset);
>  
>  extern void addrconf_prefix_rcv(struct net_device *dev,
>  				u8 *opt, int len, bool sllao);
> diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
> index a52d864..6de509e 100644
> --- a/net/ipv6/ip6_input.c
> +++ b/net/ipv6/ip6_input.c
> @@ -212,7 +212,7 @@ resubmit:
>  			if (ipv6_addr_is_multicast(&hdr->daddr) &&
>  			    !ipv6_chk_mcast_addr(skb->dev, &hdr->daddr,
>  			    &hdr->saddr) &&
> -			    !ipv6_is_mld(skb, nexthdr))
> +			    !ipv6_is_mld(skb, nexthdr, 0))
>  				goto discard;
>  		}
>  		if (!(ipprot->flags & INET6_PROTO_NOPOLICY) &&
> @@ -283,7 +283,6 @@ int ip6_mc_input(struct sk_buff *skb)
>  		if (unlikely(opt->ra)) {
>  			/* Check if this is a mld message */
>  			u8 *ptr = skb_network_header(skb) + opt->ra;
> -			struct icmp6hdr *icmp6;
>  			u8 nexthdr = hdr->nexthdr;
>  			__be16 frag_off;
>  			int offset;
> @@ -303,24 +302,10 @@ int ip6_mc_input(struct sk_buff *skb)
>  				if (offset < 0)
>  					goto out;
>  
> -				if (nexthdr != IPPROTO_ICMPV6)
> +				if (!ipv6_is_mld(skb, offset, nexthdr))
>  					goto out;
>  
> -				if (!pskb_may_pull(skb, (skb_network_header(skb) +
> -						   offset + 1 - skb->data)))
> -					goto out;
> -
> -				icmp6 = (struct icmp6hdr *)(skb_network_header(skb) + offset);
> -
> -				switch (icmp6->icmp6_type) {
> -				case ICMPV6_MGM_QUERY:
> -				case ICMPV6_MGM_REPORT:
> -				case ICMPV6_MGM_REDUCTION:
> -				case ICMPV6_MLD2_REPORT:
> -					deliver = true;
> -					break;
> -				}
> -				goto out;
> +				deliver = true;
>  			}
>  			/* unknown RA - process it normally */
>  		}
> diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
> index 28dfa5f..b529c27 100644
> --- a/net/ipv6/mcast.c
> +++ b/net/ipv6/mcast.c
> @@ -937,14 +937,14 @@ int ipv6_dev_mc_dec(struct net_device *dev, const struct in6_addr *addr)
>  /*
>   * identify MLD packets for MLD filter exceptions
>   */
> -bool ipv6_is_mld(struct sk_buff *skb, int nexthdr)
> +bool ipv6_is_mld(struct sk_buff *skb, int nexthdr, int offset)
>  {
>  	struct icmp6hdr *pic;
>  
>  	if (nexthdr != IPPROTO_ICMPV6)
>  		return false;
>  
> -	if (!pskb_may_pull(skb, sizeof(struct icmp6hdr)))
> +	if (!pskb_may_pull(skb, offset + sizeof(struct icmp6hdr)))
>  		return false;
>  
>  	pic = icmp6_hdr(skb);
> 

Sorry I found some bugs, I'll repost.

--yoshfuji

^ permalink raw reply

* RE: [RFC PATCH] net: phy: remove flags argument from phy_{attach,connect,connect_direct}
From: Kosta Zertsekel @ 2013-01-13  8:54 UTC (permalink / raw)
  To: Florian Fainelli, netdev@vger.kernel.org
  Cc: davem@davemloft.net, afleming@gmail.com
In-Reply-To: <1357903907-16049-1-git-send-email-florian@openwrt.org>

> The flags argument of the phy_{attach,connect,connect_direct} functions is used to assign a struct phy_device dev_flags with the value passed.
> All callers but the tg3 driver pass the flag 0, which results in the underlying PHY drivers in drivers/net/phy/ not being able to actually use any
> of the flags they would check in dev_flags. This patch gets rid of the flags argument, and passes phydev->dev_flags to the internal PHY library call
> phy_attach_direct() such that drivers which actually modify a phy device dev_flags get the value preserved for use by the underly phy driver.

Acked by konszert@marvell.com.

It seems this patch should be reposted on linux-kernel@vger.kernel.org list for wider approval, isn't it?
Anyway, I think it is prudent to abandon my narrow patch that fixed only phy_attach() in mv643xx network driver.

--- KostaZ

^ permalink raw reply

* [PATCH net-next] ipv6: Make ipv6_is_mld() inline and use it from ip6_mc_input().
From: YOSHIFUJI Hideaki @ 2013-01-13 10:09 UTC (permalink / raw)
  To: davem, netdev; +Cc: yoshfuji

Move generalized version of ipv6_is_mld() to header,
and use it from ip6_mc_input().

Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 include/net/addrconf.h |   26 +++++++++++++++++++++++++-
 net/ipv6/ip6_input.c   |   21 +++------------------
 net/ipv6/mcast.c       |   27 ---------------------------
 3 files changed, 28 insertions(+), 46 deletions(-)

diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index df4ef94..7cd14c0 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -150,7 +150,31 @@ extern void addrconf_dad_failure(struct inet6_ifaddr *ifp);
 extern bool ipv6_chk_mcast_addr(struct net_device *dev,
 				const struct in6_addr *group,
 				const struct in6_addr *src_addr);
-extern bool ipv6_is_mld(struct sk_buff *skb, int nexthdr);
+
+/*
+ * identify MLD packets for MLD filter exceptions
+ */
+static inline bool ipv6_is_mld(struct sk_buff *skb, int nexthdr, int offset)
+{
+	struct icmp6hdr *hdr;
+
+	if (nexthdr != IPPROTO_ICMPV6 ||
+	    !pskb_network_may_pull(skb, offset + sizeof(struct icmp6hdr)))
+		return false;
+
+	hdr = (struct icmp6hdr *)(skb_network_header(skb) + offset);
+
+	switch (hdr->icmp6_type) {
+	case ICMPV6_MGM_QUERY:
+	case ICMPV6_MGM_REPORT:
+	case ICMPV6_MGM_REDUCTION:
+	case ICMPV6_MLD2_REPORT:
+		return true;
+	default:
+		break;
+	}
+	return false;
+}
 
 extern void addrconf_prefix_rcv(struct net_device *dev,
 				u8 *opt, int len, bool sllao);
diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
index a52d864..2ccd35e 100644
--- a/net/ipv6/ip6_input.c
+++ b/net/ipv6/ip6_input.c
@@ -212,7 +212,7 @@ resubmit:
 			if (ipv6_addr_is_multicast(&hdr->daddr) &&
 			    !ipv6_chk_mcast_addr(skb->dev, &hdr->daddr,
 			    &hdr->saddr) &&
-			    !ipv6_is_mld(skb, nexthdr))
+			    !ipv6_is_mld(skb, nexthdr, skb_network_header_len(skb)))
 				goto discard;
 		}
 		if (!(ipprot->flags & INET6_PROTO_NOPOLICY) &&
@@ -283,7 +283,6 @@ int ip6_mc_input(struct sk_buff *skb)
 		if (unlikely(opt->ra)) {
 			/* Check if this is a mld message */
 			u8 *ptr = skb_network_header(skb) + opt->ra;
-			struct icmp6hdr *icmp6;
 			u8 nexthdr = hdr->nexthdr;
 			__be16 frag_off;
 			int offset;
@@ -303,24 +302,10 @@ int ip6_mc_input(struct sk_buff *skb)
 				if (offset < 0)
 					goto out;
 
-				if (nexthdr != IPPROTO_ICMPV6)
+				if (!ipv6_is_mld(skb, nexthdr, offset))
 					goto out;
 
-				if (!pskb_may_pull(skb, (skb_network_header(skb) +
-						   offset + 1 - skb->data)))
-					goto out;
-
-				icmp6 = (struct icmp6hdr *)(skb_network_header(skb) + offset);
-
-				switch (icmp6->icmp6_type) {
-				case ICMPV6_MGM_QUERY:
-				case ICMPV6_MGM_REPORT:
-				case ICMPV6_MGM_REDUCTION:
-				case ICMPV6_MLD2_REPORT:
-					deliver = true;
-					break;
-				}
-				goto out;
+				deliver = true;
 			}
 			/* unknown RA - process it normally */
 		}
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 28dfa5f..8237ee1 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -935,33 +935,6 @@ int ipv6_dev_mc_dec(struct net_device *dev, const struct in6_addr *addr)
 }
 
 /*
- * identify MLD packets for MLD filter exceptions
- */
-bool ipv6_is_mld(struct sk_buff *skb, int nexthdr)
-{
-	struct icmp6hdr *pic;
-
-	if (nexthdr != IPPROTO_ICMPV6)
-		return false;
-
-	if (!pskb_may_pull(skb, sizeof(struct icmp6hdr)))
-		return false;
-
-	pic = icmp6_hdr(skb);
-
-	switch (pic->icmp6_type) {
-	case ICMPV6_MGM_QUERY:
-	case ICMPV6_MGM_REPORT:
-	case ICMPV6_MGM_REDUCTION:
-	case ICMPV6_MLD2_REPORT:
-		return true;
-	default:
-		break;
-	}
-	return false;
-}
-
-/*
  *	check if the interface/address pair is valid
  */
 bool ipv6_chk_mcast_addr(struct net_device *dev, const struct in6_addr *group,
-- 
1.7.9.5

^ permalink raw reply related

* Re: [PATCH 1/3] ssb: add missing method ssb_gige_get_macaddr
From: Michael Büsch @ 2013-01-13  9:47 UTC (permalink / raw)
  To: David Miller; +Cc: hauke, mcarlson, mchan, netdev
In-Reply-To: <20130112.153211.562541025115995797.davem@davemloft.net>

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

On Sat, 12 Jan 2013 15:32:11 -0800 (PST)
David Miller <davem@davemloft.net> wrote:

> From: Hauke Mehrtens <hauke@hauke-m.de>
> Date: Sat, 12 Jan 2013 21:01:43 +0100
> 
> > When CONFIG_SSB_DRIVER_GIGE is not set the header does not provide the
> > needed method.
> > 
> > Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
> 
> This isn't right.
> 
> You can't implement this function in a way that the caller cannot
> determine that it didn't actually do anything.
> 
> You either have to protect calls with ifdefs or make this routine
> return an error indication, in response to which the caller can
> set a random ethernet address or take some other corrective action.

tg3 uses is_valid_ether_addr() afterwards anyway. An error code return value
could be added, but it didn't seem necessary.

-- 
Greetings, Michael.

PGP: 908D8B0E

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

^ permalink raw reply

* Re: [RFC davem] revert: net: Make skb->skb_iif always track skb->dev
From: Oliver Hartkopp @ 2013-01-13 11:01 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20130112.133630.257139657732337147.davem@davemloft.net>

On 12.01.2013 22:36, David Miller wrote:

> As per your problem with CAN, that's also rediculous.  You have an SKB
> control block in skb->cb[] that you can put whatever values with
> whatever semantics you want.
> 
> Use it.


I'm not writing a RFC to you, when i'm not sure having checked several options
before.

In the tx path already net/sched is using cb[] for it's purposes.
Adding the information somewhere at the end of cb[] to not interfere with
sched becomes tricky. See users of qdisc_cb_private_validate().

So the 'incoming interface index' could be stored safely in the tx path in

- skb->data
- skb_shared_info
- skb_iif

skb_iif was the obvious choice then. And the question why a seven year old
if-statement in net/core/dev.c has been removed must be allowed.

Networking is not only IP networking.

Oliver

^ permalink raw reply

* Re: [PATCH V3 2/2] vhost: handle polling errors
From: Michael S. Tsirkin @ 2013-01-13 11:04 UTC (permalink / raw)
  To: Jason Wang; +Cc: kvm, eric.dumazet, netdev, linux-kernel, virtualization, davem
In-Reply-To: <50EAE400.6030301@redhat.com>

On Mon, Jan 07, 2013 at 11:04:32PM +0800, Jason Wang wrote:
> On 01/07/2013 10:55 PM, Michael S. Tsirkin wrote:
> > On Mon, Jan 07, 2013 at 12:38:17PM +0800, Jason Wang wrote:
> >> On 01/06/2013 09:22 PM, Michael S. Tsirkin wrote:
> >>> On Sun, Jan 06, 2013 at 03:18:38PM +0800, Jason Wang wrote:
> >>>> Polling errors were ignored by vhost/vhost_net, this may lead to crash when
> >>>> trying to remove vhost from waitqueue when after the polling is failed. Solve
> >>>> this problem by:
> >>>>
> >>>> - checking the poll->wqh before trying to remove from waitqueue
> >>>> - report an error when poll() returns a POLLERR in vhost_start_poll()
> >>>> - report an error when vhost_start_poll() fails in
> >>>>   vhost_vring_ioctl()/vhost_net_set_backend() which is used to notify the
> >>>>   failure to userspace.
> >>>> - report an error in the data path in vhost_net when meet polling errors.
> >>>>
> >>>> After those changes, we can safely drop the tx polling state in vhost_net since
> >>>> it was replaced by the checking of poll->wqh.
> >>>>
> >>>> Signed-off-by: Jason Wang <jasowang@redhat.com>
> >>>> ---
> >>>>  drivers/vhost/net.c   |   74 ++++++++++++++++--------------------------------
> >>>>  drivers/vhost/vhost.c |   31 +++++++++++++++-----
> >>>>  drivers/vhost/vhost.h |    2 +-
> >>>>  3 files changed, 49 insertions(+), 58 deletions(-)
> >>>>
> >>>> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> >>>> index d10ad6f..125c1e5 100644
> >>>> --- a/drivers/vhost/net.c
> >>>> +++ b/drivers/vhost/net.c
> >>>> @@ -64,20 +64,10 @@ enum {
> >>>>  	VHOST_NET_VQ_MAX = 2,
> >>>>  };
> >>>>  
> >>>> -enum vhost_net_poll_state {
> >>>> -	VHOST_NET_POLL_DISABLED = 0,
> >>>> -	VHOST_NET_POLL_STARTED = 1,
> >>>> -	VHOST_NET_POLL_STOPPED = 2,
> >>>> -};
> >>>> -
> >>>>  struct vhost_net {
> >>>>  	struct vhost_dev dev;
> >>>>  	struct vhost_virtqueue vqs[VHOST_NET_VQ_MAX];
> >>>>  	struct vhost_poll poll[VHOST_NET_VQ_MAX];
> >>>> -	/* Tells us whether we are polling a socket for TX.
> >>>> -	 * We only do this when socket buffer fills up.
> >>>> -	 * Protected by tx vq lock. */
> >>>> -	enum vhost_net_poll_state tx_poll_state;
> >>>>  	/* Number of TX recently submitted.
> >>>>  	 * Protected by tx vq lock. */
> >>>>  	unsigned tx_packets;
> >>>> @@ -155,24 +145,6 @@ static void copy_iovec_hdr(const struct iovec *from, struct iovec *to,
> >>>>  	}
> >>>>  }
> >>>>  
> >>>> -/* Caller must have TX VQ lock */
> >>>> -static void tx_poll_stop(struct vhost_net *net)
> >>>> -{
> >>>> -	if (likely(net->tx_poll_state != VHOST_NET_POLL_STARTED))
> >>>> -		return;
> >>>> -	vhost_poll_stop(net->poll + VHOST_NET_VQ_TX);
> >>>> -	net->tx_poll_state = VHOST_NET_POLL_STOPPED;
> >>>> -}
> >>>> -
> >>>> -/* Caller must have TX VQ lock */
> >>>> -static void tx_poll_start(struct vhost_net *net, struct socket *sock)
> >>>> -{
> >>>> -	if (unlikely(net->tx_poll_state != VHOST_NET_POLL_STOPPED))
> >>>> -		return;
> >>>> -	vhost_poll_start(net->poll + VHOST_NET_VQ_TX, sock->file);
> >>>> -	net->tx_poll_state = VHOST_NET_POLL_STARTED;
> >>>> -}
> >>>> -
> >>>>  /* In case of DMA done not in order in lower device driver for some reason.
> >>>>   * upend_idx is used to track end of used idx, done_idx is used to track head
> >>>>   * of used idx. Once lower device DMA done contiguously, we will signal KVM
> >>>> @@ -227,6 +199,7 @@ static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success)
> >>>>  static void handle_tx(struct vhost_net *net)
> >>>>  {
> >>>>  	struct vhost_virtqueue *vq = &net->dev.vqs[VHOST_NET_VQ_TX];
> >>>> +	struct vhost_poll *poll = net->poll + VHOST_NET_VQ_TX;
> >>>>  	unsigned out, in, s;
> >>>>  	int head;
> >>>>  	struct msghdr msg = {
> >>>> @@ -252,7 +225,8 @@ static void handle_tx(struct vhost_net *net)
> >>>>  	wmem = atomic_read(&sock->sk->sk_wmem_alloc);
> >>>>  	if (wmem >= sock->sk->sk_sndbuf) {
> >>>>  		mutex_lock(&vq->mutex);
> >>>> -		tx_poll_start(net, sock);
> >>>> +		if (vhost_poll_start(poll, sock->file))
> >>>> +			vq_err(vq, "Fail to start TX polling\n");
> >>> s/Fail/Failed/
> >>>
> >>> A question though: how can this happen? Could you clarify please?
> >>> Maybe we can find a way to prevent this error?
> >> Two conditions I think this can happen:
> >>
> >> 1) a buggy userspace disable a queue through TUNSETQUEUE
> >> 2) the net device were gone
> >>
> >> For 1, looks like we can delay the disabling until the refcnt goes to
> >> zero. For 2 may needs more changes.
> > I'd expect keeping a socket reference would prevent both issues.
> > Doesn't it?
> 
> Doesn't work for 2 I think, the socket didn't hold a refcnt of the
> device, so the device can go away at anytime.

Hmm I don't really understand.
All we care about is that socket is around no?
Could you please show how a problematic case can
be triggered?


> Although we can change
> this, but it's the behaviour before multiqueue support.
> >
> >> Not sure it's worth to do this work,
> >> maybe a warning is enough just like other failure.
> > With other failures, you normally can correct the error then
> > kick to have it restart. This is soomething thagt would not
> > work here.
> 
> If userspace is wrote correctly, (e.g passing a fd with correct state)
> it can also be corrected.

What I mean is that since we don't poll
the backend, there appears to be no way to
recover. So I'd prefer that we prevent
userspace from creating this broken configuration
when ring is running.


> >
> >>>>  		mutex_unlock(&vq->mutex);
> >>>>  		return;
> >>>>  	}
> >>>> @@ -261,7 +235,7 @@ static void handle_tx(struct vhost_net *net)
> >>>>  	vhost_disable_notify(&net->dev, vq);
> >>>>  
> >>>>  	if (wmem < sock->sk->sk_sndbuf / 2)
> >>>> -		tx_poll_stop(net);
> >>>> +		vhost_poll_stop(poll);
> >>>>  	hdr_size = vq->vhost_hlen;
> >>>>  	zcopy = vq->ubufs;
> >>>>  
> >>>> @@ -283,8 +257,10 @@ static void handle_tx(struct vhost_net *net)
> >>>>  
> >>>>  			wmem = atomic_read(&sock->sk->sk_wmem_alloc);
> >>>>  			if (wmem >= sock->sk->sk_sndbuf * 3 / 4) {
> >>>> -				tx_poll_start(net, sock);
> >>>> -				set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
> >>>> +				if (vhost_poll_start(poll, sock->file))
> >>>> +					vq_err(vq, "Fail to start TX polling\n");
> >>>> +				else
> >>>> +					set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
> >>>>  				break;
> >>>>  			}
> >>>>  			/* If more outstanding DMAs, queue the work.
> >>>> @@ -294,8 +270,10 @@ static void handle_tx(struct vhost_net *net)
> >>>>  				    (vq->upend_idx - vq->done_idx) :
> >>>>  				    (vq->upend_idx + UIO_MAXIOV - vq->done_idx);
> >>>>  			if (unlikely(num_pends > VHOST_MAX_PEND)) {
> >>>> -				tx_poll_start(net, sock);
> >>>> -				set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
> >>>> +				if (vhost_poll_start(poll, sock->file))
> >>>> +					vq_err(vq, "Fail to start TX polling\n");
> >>>> +				else
> >>>> +					set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
> >>>>  				break;
> >>>>  			}
> >>>>  			if (unlikely(vhost_enable_notify(&net->dev, vq))) {
> >>>> @@ -360,7 +338,8 @@ static void handle_tx(struct vhost_net *net)
> >>>>  			}
> >>>>  			vhost_discard_vq_desc(vq, 1);
> >>>>  			if (err == -EAGAIN || err == -ENOBUFS)
> >>>> -				tx_poll_start(net, sock);
> >>>> +				if (vhost_poll_start(poll, sock->file))
> >>>> +					vq_err(vq, "Fail to start TX polling\n");
> >>>>  			break;
> >>>>  		}
> >>>>  		if (err != len)
> >>>> @@ -623,7 +602,6 @@ static int vhost_net_open(struct inode *inode, struct file *f)
> >>>>  
> >>>>  	vhost_poll_init(n->poll + VHOST_NET_VQ_TX, handle_tx_net, POLLOUT, dev);
> >>>>  	vhost_poll_init(n->poll + VHOST_NET_VQ_RX, handle_rx_net, POLLIN, dev);
> >>>> -	n->tx_poll_state = VHOST_NET_POLL_DISABLED;
> >>>>  
> >>>>  	f->private_data = n;
> >>>>  
> >>>> @@ -633,29 +611,25 @@ static int vhost_net_open(struct inode *inode, struct file *f)
> >>>>  static void vhost_net_disable_vq(struct vhost_net *n,
> >>>>  				 struct vhost_virtqueue *vq)
> >>>>  {
> >>>> +	struct vhost_poll *poll = n->poll + (vq - n->vqs);
> >>>> +
> >>>>  	if (!vq->private_data)
> >>>>  		return;
> >>>> -	if (vq == n->vqs + VHOST_NET_VQ_TX) {
> >>>> -		tx_poll_stop(n);
> >>>> -		n->tx_poll_state = VHOST_NET_POLL_DISABLED;
> >>>> -	} else
> >>>> -		vhost_poll_stop(n->poll + VHOST_NET_VQ_RX);
> >>>> +	vhost_poll_stop(poll);
> >>>>  }
> >>>>  
> >>>> -static void vhost_net_enable_vq(struct vhost_net *n,
> >>>> +static int vhost_net_enable_vq(struct vhost_net *n,
> >>>>  				struct vhost_virtqueue *vq)
> >>>>  {
> >>>>  	struct socket *sock;
> >>>> +	struct vhost_poll *poll = n->poll + (vq - n->vqs);
> >>>>  
> >>>>  	sock = rcu_dereference_protected(vq->private_data,
> >>>>  					 lockdep_is_held(&vq->mutex));
> >>>>  	if (!sock)
> >>>> -		return;
> >>>> -	if (vq == n->vqs + VHOST_NET_VQ_TX) {
> >>>> -		n->tx_poll_state = VHOST_NET_POLL_STOPPED;
> >>>> -		tx_poll_start(n, sock);
> >>>> -	} else
> >>>> -		vhost_poll_start(n->poll + VHOST_NET_VQ_RX, sock->file);
> >>>> +		return 0;
> >>>> +
> >>>> +	return vhost_poll_start(poll, sock->file);
> >>>>  }
> >>>>  
> >>>>  static struct socket *vhost_net_stop_vq(struct vhost_net *n,
> >>>> @@ -833,7 +807,9 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
> >>>>  		r = vhost_init_used(vq);
> >>>>  		if (r)
> >>>>  			goto err_used;
> >>>> -		vhost_net_enable_vq(n, vq);
> >>>> +		r = vhost_net_enable_vq(n, vq);
> >>>> +		if (r)
> >>>> +			goto err_used;
> >>>>  
> >>>>  		oldubufs = vq->ubufs;
> >>>>  		vq->ubufs = ubufs;
> >>>> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> >>>> index 34389f7..5c7a466 100644
> >>>> --- a/drivers/vhost/vhost.c
> >>>> +++ b/drivers/vhost/vhost.c
> >>>> @@ -77,26 +77,41 @@ void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
> >>>>  	init_poll_funcptr(&poll->table, vhost_poll_func);
> >>>>  	poll->mask = mask;
> >>>>  	poll->dev = dev;
> >>>> +	poll->wqh = NULL;
> >>>>  
> >>>>  	vhost_work_init(&poll->work, fn);
> >>>>  }
> >>>>  
> >>>> +/* Stop polling a file. After this function returns, it becomes safe to drop the
> >>>> + * file reference. You must also flush afterwards. */
> >>>> +void vhost_poll_stop(struct vhost_poll *poll)
> >>>> +{
> >>>> +	if (poll->wqh) {
> >>>> +		remove_wait_queue(poll->wqh, &poll->wait);
> >>>> +		poll->wqh = NULL;
> >>>> +	}
> >>>> +}
> >>>> +
> >>>>  /* Start polling a file. We add ourselves to file's wait queue. The caller must
> >>>>   * keep a reference to a file until after vhost_poll_stop is called. */
> >>>> -void vhost_poll_start(struct vhost_poll *poll, struct file *file)
> >>>> +int vhost_poll_start(struct vhost_poll *poll, struct file *file)
> >>>>  {
> >>>>  	unsigned long mask;
> >>>> +	int ret = 0;
> >>>> +
> >>>> +	if (poll->wqh)
> >>>> +		return -EBUSY;
> >>>>  
> >>> I think this should return success: we are already polling.
> >>> Otherwise this would trigger a bug below I think.
> >> Ok.
> >>>>  	mask = file->f_op->poll(file, &poll->table);
> >>>>  	if (mask)
> >>>>  		vhost_poll_wakeup(&poll->wait, 0, 0, (void *)mask);
> >>>> -}
> >>>>  
> >>>> -/* Stop polling a file. After this function returns, it becomes safe to drop the
> >>>> - * file reference. You must also flush afterwards. */
> >>>> -void vhost_poll_stop(struct vhost_poll *poll)
> >>>> -{
> >>>> -	remove_wait_queue(poll->wqh, &poll->wait);
> >>>> +	if (mask & POLLERR) {
> >>>> +		ret = -EINVAL;
> >>>> +		vhost_poll_stop(poll);
> >>>> +	}
> >>>> +
> >>>> +	return ret;
> >>>>  }
> >>>>  
> >>>>  static bool vhost_work_seq_done(struct vhost_dev *dev, struct vhost_work *work,
> >>>> @@ -792,7 +807,7 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp)
> >>>>  		fput(filep);
> >>>>  
> >>>>  	if (pollstart && vq->handle_kick)
> >>>> -		vhost_poll_start(&vq->poll, vq->kick);
> >>>> +		r = vhost_poll_start(&vq->poll, vq->kick);
> >>>>  
> >>>>  	mutex_unlock(&vq->mutex);
> >>>>  
> >>>> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> >>>> index 2639c58..17261e2 100644
> >>>> --- a/drivers/vhost/vhost.h
> >>>> +++ b/drivers/vhost/vhost.h
> >>>> @@ -42,7 +42,7 @@ void vhost_work_queue(struct vhost_dev *dev, struct vhost_work *work);
> >>>>  
> >>>>  void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
> >>>>  		     unsigned long mask, struct vhost_dev *dev);
> >>>> -void vhost_poll_start(struct vhost_poll *poll, struct file *file);
> >>>> +int vhost_poll_start(struct vhost_poll *poll, struct file *file);
> >>>>  void vhost_poll_stop(struct vhost_poll *poll);
> >>>>  void vhost_poll_flush(struct vhost_poll *poll);
> >>>>  void vhost_poll_queue(struct vhost_poll *poll);
> >>>> -- 
> >>>> 1.7.1
> >>> --
> >>> 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
> > --
> > To unsubscribe from this list: send the line "unsubscribe kvm" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH V3 2/2] vhost: handle polling errors
From: Michael S. Tsirkin @ 2013-01-13 11:10 UTC (permalink / raw)
  To: Jason Wang; +Cc: kvm, eric.dumazet, netdev, linux-kernel, virtualization, davem
In-Reply-To: <50EAE400.6030301@redhat.com>

On Mon, Jan 07, 2013 at 11:04:32PM +0800, Jason Wang wrote:
> On 01/07/2013 10:55 PM, Michael S. Tsirkin wrote:
> > On Mon, Jan 07, 2013 at 12:38:17PM +0800, Jason Wang wrote:
> >> On 01/06/2013 09:22 PM, Michael S. Tsirkin wrote:
> >>> On Sun, Jan 06, 2013 at 03:18:38PM +0800, Jason Wang wrote:
> >>>> Polling errors were ignored by vhost/vhost_net, this may lead to crash when
> >>>> trying to remove vhost from waitqueue when after the polling is failed. Solve
> >>>> this problem by:
> >>>>
> >>>> - checking the poll->wqh before trying to remove from waitqueue
> >>>> - report an error when poll() returns a POLLERR in vhost_start_poll()
> >>>> - report an error when vhost_start_poll() fails in
> >>>>   vhost_vring_ioctl()/vhost_net_set_backend() which is used to notify the
> >>>>   failure to userspace.
> >>>> - report an error in the data path in vhost_net when meet polling errors.
> >>>>
> >>>> After those changes, we can safely drop the tx polling state in vhost_net since
> >>>> it was replaced by the checking of poll->wqh.
> >>>>
> >>>> Signed-off-by: Jason Wang <jasowang@redhat.com>
> >>>> ---
> >>>>  drivers/vhost/net.c   |   74 ++++++++++++++++--------------------------------
> >>>>  drivers/vhost/vhost.c |   31 +++++++++++++++-----
> >>>>  drivers/vhost/vhost.h |    2 +-
> >>>>  3 files changed, 49 insertions(+), 58 deletions(-)
> >>>>
> >>>> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> >>>> index d10ad6f..125c1e5 100644
> >>>> --- a/drivers/vhost/net.c
> >>>> +++ b/drivers/vhost/net.c
> >>>> @@ -64,20 +64,10 @@ enum {
> >>>>  	VHOST_NET_VQ_MAX = 2,
> >>>>  };
> >>>>  
> >>>> -enum vhost_net_poll_state {
> >>>> -	VHOST_NET_POLL_DISABLED = 0,
> >>>> -	VHOST_NET_POLL_STARTED = 1,
> >>>> -	VHOST_NET_POLL_STOPPED = 2,
> >>>> -};
> >>>> -
> >>>>  struct vhost_net {
> >>>>  	struct vhost_dev dev;
> >>>>  	struct vhost_virtqueue vqs[VHOST_NET_VQ_MAX];
> >>>>  	struct vhost_poll poll[VHOST_NET_VQ_MAX];
> >>>> -	/* Tells us whether we are polling a socket for TX.
> >>>> -	 * We only do this when socket buffer fills up.
> >>>> -	 * Protected by tx vq lock. */
> >>>> -	enum vhost_net_poll_state tx_poll_state;
> >>>>  	/* Number of TX recently submitted.
> >>>>  	 * Protected by tx vq lock. */
> >>>>  	unsigned tx_packets;
> >>>> @@ -155,24 +145,6 @@ static void copy_iovec_hdr(const struct iovec *from, struct iovec *to,
> >>>>  	}
> >>>>  }
> >>>>  
> >>>> -/* Caller must have TX VQ lock */
> >>>> -static void tx_poll_stop(struct vhost_net *net)
> >>>> -{
> >>>> -	if (likely(net->tx_poll_state != VHOST_NET_POLL_STARTED))
> >>>> -		return;
> >>>> -	vhost_poll_stop(net->poll + VHOST_NET_VQ_TX);
> >>>> -	net->tx_poll_state = VHOST_NET_POLL_STOPPED;
> >>>> -}
> >>>> -
> >>>> -/* Caller must have TX VQ lock */
> >>>> -static void tx_poll_start(struct vhost_net *net, struct socket *sock)
> >>>> -{
> >>>> -	if (unlikely(net->tx_poll_state != VHOST_NET_POLL_STOPPED))
> >>>> -		return;
> >>>> -	vhost_poll_start(net->poll + VHOST_NET_VQ_TX, sock->file);
> >>>> -	net->tx_poll_state = VHOST_NET_POLL_STARTED;
> >>>> -}
> >>>> -
> >>>>  /* In case of DMA done not in order in lower device driver for some reason.
> >>>>   * upend_idx is used to track end of used idx, done_idx is used to track head
> >>>>   * of used idx. Once lower device DMA done contiguously, we will signal KVM
> >>>> @@ -227,6 +199,7 @@ static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success)
> >>>>  static void handle_tx(struct vhost_net *net)
> >>>>  {
> >>>>  	struct vhost_virtqueue *vq = &net->dev.vqs[VHOST_NET_VQ_TX];
> >>>> +	struct vhost_poll *poll = net->poll + VHOST_NET_VQ_TX;
> >>>>  	unsigned out, in, s;
> >>>>  	int head;
> >>>>  	struct msghdr msg = {
> >>>> @@ -252,7 +225,8 @@ static void handle_tx(struct vhost_net *net)
> >>>>  	wmem = atomic_read(&sock->sk->sk_wmem_alloc);
> >>>>  	if (wmem >= sock->sk->sk_sndbuf) {
> >>>>  		mutex_lock(&vq->mutex);
> >>>> -		tx_poll_start(net, sock);
> >>>> +		if (vhost_poll_start(poll, sock->file))
> >>>> +			vq_err(vq, "Fail to start TX polling\n");
> >>> s/Fail/Failed/
> >>>
> >>> A question though: how can this happen? Could you clarify please?
> >>> Maybe we can find a way to prevent this error?
> >> Two conditions I think this can happen:
> >>
> >> 1) a buggy userspace disable a queue through TUNSETQUEUE
> >> 2) the net device were gone
> >>
> >> For 1, looks like we can delay the disabling until the refcnt goes to
> >> zero. For 2 may needs more changes.
> > I'd expect keeping a socket reference would prevent both issues.
> > Doesn't it?
> 
> Doesn't work for 2 I think, the socket didn't hold a refcnt of the
> device, so the device can go away at anytime. Although we can change
> this, but it's the behaviour before multiqueue support.

Hmm there's one scenario that does seem to
trigger this: queue can get disabled
and then poll fails.

Is this the only issue?


> >
> >> Not sure it's worth to do this work,
> >> maybe a warning is enough just like other failure.
> > With other failures, you normally can correct the error then
> > kick to have it restart. This is soomething thagt would not
> > work here.
> 
> If userspace is wrote correctly, (e.g passing a fd with correct state)
> it can also be corrected.
> >
> >>>>  		mutex_unlock(&vq->mutex);
> >>>>  		return;
> >>>>  	}
> >>>> @@ -261,7 +235,7 @@ static void handle_tx(struct vhost_net *net)
> >>>>  	vhost_disable_notify(&net->dev, vq);
> >>>>  
> >>>>  	if (wmem < sock->sk->sk_sndbuf / 2)
> >>>> -		tx_poll_stop(net);
> >>>> +		vhost_poll_stop(poll);
> >>>>  	hdr_size = vq->vhost_hlen;
> >>>>  	zcopy = vq->ubufs;
> >>>>  
> >>>> @@ -283,8 +257,10 @@ static void handle_tx(struct vhost_net *net)
> >>>>  
> >>>>  			wmem = atomic_read(&sock->sk->sk_wmem_alloc);
> >>>>  			if (wmem >= sock->sk->sk_sndbuf * 3 / 4) {
> >>>> -				tx_poll_start(net, sock);
> >>>> -				set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
> >>>> +				if (vhost_poll_start(poll, sock->file))
> >>>> +					vq_err(vq, "Fail to start TX polling\n");
> >>>> +				else
> >>>> +					set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
> >>>>  				break;
> >>>>  			}
> >>>>  			/* If more outstanding DMAs, queue the work.
> >>>> @@ -294,8 +270,10 @@ static void handle_tx(struct vhost_net *net)
> >>>>  				    (vq->upend_idx - vq->done_idx) :
> >>>>  				    (vq->upend_idx + UIO_MAXIOV - vq->done_idx);
> >>>>  			if (unlikely(num_pends > VHOST_MAX_PEND)) {
> >>>> -				tx_poll_start(net, sock);
> >>>> -				set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
> >>>> +				if (vhost_poll_start(poll, sock->file))
> >>>> +					vq_err(vq, "Fail to start TX polling\n");
> >>>> +				else
> >>>> +					set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
> >>>>  				break;
> >>>>  			}
> >>>>  			if (unlikely(vhost_enable_notify(&net->dev, vq))) {
> >>>> @@ -360,7 +338,8 @@ static void handle_tx(struct vhost_net *net)
> >>>>  			}
> >>>>  			vhost_discard_vq_desc(vq, 1);
> >>>>  			if (err == -EAGAIN || err == -ENOBUFS)
> >>>> -				tx_poll_start(net, sock);
> >>>> +				if (vhost_poll_start(poll, sock->file))
> >>>> +					vq_err(vq, "Fail to start TX polling\n");
> >>>>  			break;
> >>>>  		}
> >>>>  		if (err != len)
> >>>> @@ -623,7 +602,6 @@ static int vhost_net_open(struct inode *inode, struct file *f)
> >>>>  
> >>>>  	vhost_poll_init(n->poll + VHOST_NET_VQ_TX, handle_tx_net, POLLOUT, dev);
> >>>>  	vhost_poll_init(n->poll + VHOST_NET_VQ_RX, handle_rx_net, POLLIN, dev);
> >>>> -	n->tx_poll_state = VHOST_NET_POLL_DISABLED;
> >>>>  
> >>>>  	f->private_data = n;
> >>>>  
> >>>> @@ -633,29 +611,25 @@ static int vhost_net_open(struct inode *inode, struct file *f)
> >>>>  static void vhost_net_disable_vq(struct vhost_net *n,
> >>>>  				 struct vhost_virtqueue *vq)
> >>>>  {
> >>>> +	struct vhost_poll *poll = n->poll + (vq - n->vqs);
> >>>> +
> >>>>  	if (!vq->private_data)
> >>>>  		return;
> >>>> -	if (vq == n->vqs + VHOST_NET_VQ_TX) {
> >>>> -		tx_poll_stop(n);
> >>>> -		n->tx_poll_state = VHOST_NET_POLL_DISABLED;
> >>>> -	} else
> >>>> -		vhost_poll_stop(n->poll + VHOST_NET_VQ_RX);
> >>>> +	vhost_poll_stop(poll);
> >>>>  }
> >>>>  
> >>>> -static void vhost_net_enable_vq(struct vhost_net *n,
> >>>> +static int vhost_net_enable_vq(struct vhost_net *n,
> >>>>  				struct vhost_virtqueue *vq)
> >>>>  {
> >>>>  	struct socket *sock;
> >>>> +	struct vhost_poll *poll = n->poll + (vq - n->vqs);
> >>>>  
> >>>>  	sock = rcu_dereference_protected(vq->private_data,
> >>>>  					 lockdep_is_held(&vq->mutex));
> >>>>  	if (!sock)
> >>>> -		return;
> >>>> -	if (vq == n->vqs + VHOST_NET_VQ_TX) {
> >>>> -		n->tx_poll_state = VHOST_NET_POLL_STOPPED;
> >>>> -		tx_poll_start(n, sock);
> >>>> -	} else
> >>>> -		vhost_poll_start(n->poll + VHOST_NET_VQ_RX, sock->file);
> >>>> +		return 0;
> >>>> +
> >>>> +	return vhost_poll_start(poll, sock->file);
> >>>>  }
> >>>>  
> >>>>  static struct socket *vhost_net_stop_vq(struct vhost_net *n,
> >>>> @@ -833,7 +807,9 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
> >>>>  		r = vhost_init_used(vq);
> >>>>  		if (r)
> >>>>  			goto err_used;
> >>>> -		vhost_net_enable_vq(n, vq);
> >>>> +		r = vhost_net_enable_vq(n, vq);
> >>>> +		if (r)
> >>>> +			goto err_used;
> >>>>  
> >>>>  		oldubufs = vq->ubufs;
> >>>>  		vq->ubufs = ubufs;
> >>>> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> >>>> index 34389f7..5c7a466 100644
> >>>> --- a/drivers/vhost/vhost.c
> >>>> +++ b/drivers/vhost/vhost.c
> >>>> @@ -77,26 +77,41 @@ void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
> >>>>  	init_poll_funcptr(&poll->table, vhost_poll_func);
> >>>>  	poll->mask = mask;
> >>>>  	poll->dev = dev;
> >>>> +	poll->wqh = NULL;
> >>>>  
> >>>>  	vhost_work_init(&poll->work, fn);
> >>>>  }
> >>>>  
> >>>> +/* Stop polling a file. After this function returns, it becomes safe to drop the
> >>>> + * file reference. You must also flush afterwards. */
> >>>> +void vhost_poll_stop(struct vhost_poll *poll)
> >>>> +{
> >>>> +	if (poll->wqh) {
> >>>> +		remove_wait_queue(poll->wqh, &poll->wait);
> >>>> +		poll->wqh = NULL;
> >>>> +	}
> >>>> +}
> >>>> +
> >>>>  /* Start polling a file. We add ourselves to file's wait queue. The caller must
> >>>>   * keep a reference to a file until after vhost_poll_stop is called. */
> >>>> -void vhost_poll_start(struct vhost_poll *poll, struct file *file)
> >>>> +int vhost_poll_start(struct vhost_poll *poll, struct file *file)
> >>>>  {
> >>>>  	unsigned long mask;
> >>>> +	int ret = 0;
> >>>> +
> >>>> +	if (poll->wqh)
> >>>> +		return -EBUSY;
> >>>>  
> >>> I think this should return success: we are already polling.
> >>> Otherwise this would trigger a bug below I think.
> >> Ok.
> >>>>  	mask = file->f_op->poll(file, &poll->table);
> >>>>  	if (mask)
> >>>>  		vhost_poll_wakeup(&poll->wait, 0, 0, (void *)mask);
> >>>> -}
> >>>>  
> >>>> -/* Stop polling a file. After this function returns, it becomes safe to drop the
> >>>> - * file reference. You must also flush afterwards. */
> >>>> -void vhost_poll_stop(struct vhost_poll *poll)
> >>>> -{
> >>>> -	remove_wait_queue(poll->wqh, &poll->wait);
> >>>> +	if (mask & POLLERR) {
> >>>> +		ret = -EINVAL;
> >>>> +		vhost_poll_stop(poll);
> >>>> +	}
> >>>> +
> >>>> +	return ret;
> >>>>  }
> >>>>  
> >>>>  static bool vhost_work_seq_done(struct vhost_dev *dev, struct vhost_work *work,
> >>>> @@ -792,7 +807,7 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp)
> >>>>  		fput(filep);
> >>>>  
> >>>>  	if (pollstart && vq->handle_kick)
> >>>> -		vhost_poll_start(&vq->poll, vq->kick);
> >>>> +		r = vhost_poll_start(&vq->poll, vq->kick);
> >>>>  
> >>>>  	mutex_unlock(&vq->mutex);
> >>>>  
> >>>> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> >>>> index 2639c58..17261e2 100644
> >>>> --- a/drivers/vhost/vhost.h
> >>>> +++ b/drivers/vhost/vhost.h
> >>>> @@ -42,7 +42,7 @@ void vhost_work_queue(struct vhost_dev *dev, struct vhost_work *work);
> >>>>  
> >>>>  void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
> >>>>  		     unsigned long mask, struct vhost_dev *dev);
> >>>> -void vhost_poll_start(struct vhost_poll *poll, struct file *file);
> >>>> +int vhost_poll_start(struct vhost_poll *poll, struct file *file);
> >>>>  void vhost_poll_stop(struct vhost_poll *poll);
> >>>>  void vhost_poll_flush(struct vhost_poll *poll);
> >>>>  void vhost_poll_queue(struct vhost_poll *poll);
> >>>> -- 
> >>>> 1.7.1
> >>> --
> >>> 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
> > --
> > To unsubscribe from this list: send the line "unsubscribe kvm" 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

* what is the mail list address of network users ?
From: horseriver @ 2013-01-13  2:13 UTC (permalink / raw)
  To: netdev

not develop

^ permalink raw reply

* Re: [PATCH net-next] ipv6: Use ipv6_is_mld() in ip6_mc_input().
From: David Miller @ 2013-01-13 13:18 UTC (permalink / raw)
  To: yoshfuji; +Cc: netdev
In-Reply-To: <50F273C0.6080901@linux-ipv6.org>

From: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
Date: Sun, 13 Jan 2013 17:43:44 +0900

> Sorry I found some bugs, I'll repost.

You should repost the whole series, and properly number
your patches this time.

Thanks.

^ permalink raw reply

* Re: [RFC davem] revert: net: Make skb->skb_iif always track skb->dev
From: David Miller @ 2013-01-13 13:20 UTC (permalink / raw)
  To: socketcan; +Cc: netdev
In-Reply-To: <50F29421.1090405@hartkopp.net>

From: Oliver Hartkopp <socketcan@hartkopp.net>
Date: Sun, 13 Jan 2013 12:01:53 +0100

> Networking is not only IP networking.

But it is who the majority of the infrastructure is goint to be
optimized for, and this is never going to change.

^ permalink raw reply

* Re: [PATCH net-next] pkt_sched: namespace aware ifb
From: Jamal Hadi Salim @ 2013-01-13 14:44 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Benjamin LaHaise, David Miller, socketcan, netdev
In-Reply-To: <1358056199.20249.2121.camel@edumazet-glaptop>

On 13-01-13 12:49 AM, Eric Dumazet wrote:

> Could you elaborate on what could be the problem ?
>
> We hold the RTNL, so I dont think another process could possibly call
> tcf_mirred_init()
>

Eric, the point probably Ben was trying to make is not about 
synchronizing rather about which namespace has the right to that action 
config. Your change is correct for the common use of actions
but does not fix the larger picture.

At the moment a dev is owned by a specific namespace; that owns a tc 
filter that in turn owns an action. So no problem with the change you 
make if all configuration follows those rules i.e something along the 
lines of:
===
tc filter add dev eth0 parent 1: protocol ip prio 1 u32 \
match ip dst 10.0.0.229/32 flowid 1:10 \
action mirred egress redirect dev ifb0
=====

I would say most people use the above syntax.
However, there is another way to configure actions so they can be 
shared[1], example control syntax:
----
tc actions add \
action police rate 1kbit burst 90k drop index 3 \
action mirred egress mirror dev eth0 index 5
----

In such a case, the "tc actions" netlink path may be
entered from a different namespace than the one that is
using it. Then current->nsproxy->net_ns is no longer correct.

To correct this, i think what Ben points out in passing the
init() the correct namespace seem like the way to go. Feel free
to make that change - otherwise i will get to it and fix it.

cheers,
jamal

[1]
You can then have multiple filters use this action like so:
===
tc filter add dev eth0 parent 1: protocol ip prio 1 u32 \
match ip dst 10.0.0.229/32 flowid 1:10 \
action police index 3
#
tc filter add dev eth0 parent ffff: protocol ip prio 6 u32 \
match ip src 10.0.0.21/32 flowid 1:16 \
action police index 3 action mirred egress mirror dev eth0
=====

^ permalink raw reply

* Re: [PATCH net-next] pkt_sched: namespace aware ifb
From: Benjamin LaHaise @ 2013-01-13 14:59 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, socketcan, netdev
In-Reply-To: <1358056199.20249.2121.camel@edumazet-glaptop>

On Sat, Jan 12, 2013 at 09:49:59PM -0800, Eric Dumazet wrote:
> But it is working in my tests.
> 
> I added a WARN and the call stack is :
...
> Could you elaborate on what could be the problem ?
> 
> We hold the RTNL, so I dont think another process could possibly call
> tcf_mirred_init()

The locking isn't the issue, but how the network namespace is selected it.  
I've implemented some virtual router functionality using network namespaces, 
and prior to having the setns() syscall, the only way to manipulate other 
network namespaces was via socket passing between threads in different 
namespaces.  One of the optimizations in using this technique was to open a 
netlink socket in another namespace, then pass that file descriptor back to 
the main daemon.  The code could then add routes and manipulate other areas 
of the network stack via that netlink socket.

I think this technique is a valid approach for making use of network 
namespaces.  It also has the benefit of avoid the use of setns() for the 
vast majority of operations.

		-ben
-- 
"Thought is the essence of where you are now."

^ permalink raw reply

* [PATCH net-next 1/7] ipv6: Introduce ip6_flow_hdr() to fill version, tclass and flowlabel.
From: YOSHIFUJI Hideaki @ 2013-01-13 15:01 UTC (permalink / raw)
  To: davem, netdev; +Cc: yoshfuji

This is not only for readability but also for optimization.
What we do here is to build the 32bit word at the beginning of the ipv6
header (the "ip6_flow" virtual member of struct ip6_hdr in RFC3542) and
we do not need to read the tclass portion of the target buffer.

Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 include/net/ipv6.h               |    9 +++++++++
 net/ipv6/ip6_gre.c               |    6 ++----
 net/ipv6/ip6_output.c            |    8 +++-----
 net/ipv6/ip6_tunnel.c            |    4 +---
 net/ipv6/netfilter/ip6t_REJECT.c |    2 +-
 5 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index 5af66b2..fcbc646 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -547,6 +547,15 @@ static inline int ipv6_addr_diff(const struct in6_addr *a1, const struct in6_add
 extern void ipv6_select_ident(struct frag_hdr *fhdr, struct rt6_info *rt);
 
 /*
+ *	Header manipulation
+ */
+static inline void ip6_flow_hdr(struct ipv6hdr *hdr, unsigned int tclass,
+				__be32 flowlabel)
+{
+	*(__be32 *)hdr = ntohl(0x60000000 | (tclass << 20)) | flowlabel;
+}
+
+/*
  *	Prototypes exported by ipv6
  */
 
diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index c727e47..db91fe3 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -772,9 +772,7 @@ static netdev_tx_t ip6gre_xmit2(struct sk_buff *skb,
 	 *	Push down and install the IP header.
 	 */
 	ipv6h = ipv6_hdr(skb);
-	*(__be32 *)ipv6h = fl6->flowlabel | htonl(0x60000000);
-	dsfield = INET_ECN_encapsulate(0, dsfield);
-	ipv6_change_dsfield(ipv6h, ~INET_ECN_MASK, dsfield);
+	ip6_flow_hdr(ipv6h, INET_ECN_encapsulate(0, dsfield), fl6->flowlabel);
 	ipv6h->hop_limit = tunnel->parms.hop_limit;
 	ipv6h->nexthdr = proto;
 	ipv6h->saddr = fl6->saddr;
@@ -1240,7 +1238,7 @@ static int ip6gre_header(struct sk_buff *skb, struct net_device *dev,
 	struct ipv6hdr *ipv6h = (struct ipv6hdr *)skb_push(skb, t->hlen);
 	__be16 *p = (__be16 *)(ipv6h+1);
 
-	*(__be32 *)ipv6h = t->fl.u.ip6.flowlabel | htonl(0x60000000);
+	ip6_flow_hdr(ipv6h, 0, t->fl.u.ip6.flowlabel);
 	ipv6h->hop_limit = t->parms.hop_limit;
 	ipv6h->nexthdr = NEXTHDR_GRE;
 	ipv6h->saddr = t->parms.laddr;
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 5552d13..9250c69 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -216,7 +216,7 @@ int ip6_xmit(struct sock *sk, struct sk_buff *skb, struct flowi6 *fl6,
 	if (hlimit < 0)
 		hlimit = ip6_dst_hoplimit(dst);
 
-	*(__be32 *)hdr = htonl(0x60000000 | (tclass << 20)) | fl6->flowlabel;
+	ip6_flow_hdr(hdr, tclass, fl6->flowlabel);
 
 	hdr->payload_len = htons(seg_len);
 	hdr->nexthdr = proto;
@@ -267,7 +267,7 @@ int ip6_nd_hdr(struct sock *sk, struct sk_buff *skb, struct net_device *dev,
 	skb_put(skb, sizeof(struct ipv6hdr));
 	hdr = ipv6_hdr(skb);
 
-	*(__be32*)hdr = htonl(0x60000000);
+	ip6_flow_hdr(hdr, 0, 0);
 
 	hdr->payload_len = htons(len);
 	hdr->nexthdr = proto;
@@ -1548,9 +1548,7 @@ int ip6_push_pending_frames(struct sock *sk)
 	skb_reset_network_header(skb);
 	hdr = ipv6_hdr(skb);
 
-	*(__be32*)hdr = fl6->flowlabel |
-		     htonl(0x60000000 | ((int)np->cork.tclass << 20));
-
+	ip6_flow_hdr(hdr, np->cork.tclass, fl6->flowlabel);
 	hdr->hop_limit = np->cork.hop_limit;
 	hdr->nexthdr = proto;
 	hdr->saddr = fl6->saddr;
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index a14f28b..fff83cb 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -1030,9 +1030,7 @@ static int ip6_tnl_xmit2(struct sk_buff *skb,
 	skb_push(skb, sizeof(struct ipv6hdr));
 	skb_reset_network_header(skb);
 	ipv6h = ipv6_hdr(skb);
-	*(__be32*)ipv6h = fl6->flowlabel | htonl(0x60000000);
-	dsfield = INET_ECN_encapsulate(0, dsfield);
-	ipv6_change_dsfield(ipv6h, ~INET_ECN_MASK, dsfield);
+	ip6_flow_hdr(ipv6h, INET_ECN_encapsulate(0, dsfield), fl6->flowlabel);
 	ipv6h->hop_limit = t->parms.hop_limit;
 	ipv6h->nexthdr = proto;
 	ipv6h->saddr = fl6->saddr;
diff --git a/net/ipv6/netfilter/ip6t_REJECT.c b/net/ipv6/netfilter/ip6t_REJECT.c
index fd4fb34..8e75b6f 100644
--- a/net/ipv6/netfilter/ip6t_REJECT.c
+++ b/net/ipv6/netfilter/ip6t_REJECT.c
@@ -126,7 +126,7 @@ static void send_reset(struct net *net, struct sk_buff *oldskb)
 	skb_put(nskb, sizeof(struct ipv6hdr));
 	skb_reset_network_header(nskb);
 	ip6h = ipv6_hdr(nskb);
-	*(__be32 *)ip6h =  htonl(0x60000000 | (tclass << 20));
+	ip6_flow_hdr(ip6h, tclass, 0);
 	ip6h->hop_limit = ip6_dst_hoplimit(dst);
 	ip6h->nexthdr = IPPROTO_TCP;
 	ip6h->saddr = oip6h->daddr;
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH net-next 2/7] ipv6: Introduce ip6_flowinfo() to extract flowinfo (tclass + flowlabel).
From: YOSHIFUJI Hideaki @ 2013-01-13 15:01 UTC (permalink / raw)
  To: davem, netdev; +Cc: yoshfuji

Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 include/net/ipv6.h  |    5 +++++
 net/ipv6/datagram.c |    9 +++++----
 net/ipv6/route.c    |    6 +++---
 3 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index fcbc646..ed67208 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -555,6 +555,11 @@ static inline void ip6_flow_hdr(struct ipv6hdr *hdr, unsigned int tclass,
 	*(__be32 *)hdr = ntohl(0x60000000 | (tclass << 20)) | flowlabel;
 }
 
+static inline __be32 ip6_flowinfo(const struct ipv6hdr *hdr)
+{
+	return *(__be32 *)hdr & IPV6_FLOWINFO_MASK;
+}
+
 /*
  *	Prototypes exported by ipv6
  */
diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
index 56b692b..7f65df4 100644
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -360,7 +360,7 @@ int ipv6_recv_error(struct sock *sk, struct msghdr *msg, int len)
 								  struct ipv6hdr, daddr);
 			sin->sin6_addr = ip6h->daddr;
 			if (np->sndflow)
-				sin->sin6_flowinfo = *(__be32 *)ip6h & IPV6_FLOWINFO_MASK;
+				sin->sin6_flowinfo = ip6_flowinfo(ip6h);
 			if (ipv6_addr_type(&sin->sin6_addr) & IPV6_ADDR_LINKLOCAL)
 				sin->sin6_scope_id = IP6CB(skb)->iif;
 		} else {
@@ -491,9 +491,10 @@ int datagram_recv_ctl(struct sock *sk, struct msghdr *msg, struct sk_buff *skb)
 		put_cmsg(msg, SOL_IPV6, IPV6_TCLASS, sizeof(tclass), &tclass);
 	}
 
-	if (np->rxopt.bits.rxflow && (*(__be32 *)nh & IPV6_FLOWINFO_MASK)) {
-		__be32 flowinfo = *(__be32 *)nh & IPV6_FLOWINFO_MASK;
-		put_cmsg(msg, SOL_IPV6, IPV6_FLOWINFO, sizeof(flowinfo), &flowinfo);
+	if (np->rxopt.bits.rxflow) {
+		__be32 flowinfo = ip6_flowinfo((struct ipv6hdr *)nh);
+		if (flowinfo)
+			put_cmsg(msg, SOL_IPV6, IPV6_FLOWINFO, sizeof(flowinfo), &flowinfo);
 	}
 
 	/* HbH is allowed only once */
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 621b68e..6238eb5 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -994,7 +994,7 @@ void ip6_route_input(struct sk_buff *skb)
 		.flowi6_iif = skb->dev->ifindex,
 		.daddr = iph->daddr,
 		.saddr = iph->saddr,
-		.flowlabel = (* (__be32 *) iph) & IPV6_FLOWINFO_MASK,
+		.flowlabel = ip6_flowinfo(iph),
 		.flowi6_mark = skb->mark,
 		.flowi6_proto = iph->nexthdr,
 	};
@@ -1159,7 +1159,7 @@ void ip6_update_pmtu(struct sk_buff *skb, struct net *net, __be32 mtu,
 	fl6.flowi6_flags = 0;
 	fl6.daddr = iph->daddr;
 	fl6.saddr = iph->saddr;
-	fl6.flowlabel = (*(__be32 *) iph) & IPV6_FLOWINFO_MASK;
+	fl6.flowlabel = ip6_flowinfo(iph);
 
 	dst = ip6_route_output(net, NULL, &fl6);
 	if (!dst->error)
@@ -1187,7 +1187,7 @@ void ip6_redirect(struct sk_buff *skb, struct net *net, int oif, u32 mark)
 	fl6.flowi6_flags = 0;
 	fl6.daddr = iph->daddr;
 	fl6.saddr = iph->saddr;
-	fl6.flowlabel = (*(__be32 *) iph) & IPV6_FLOWINFO_MASK;
+	fl6.flowlabel = ip6_flowinfo(iph);
 
 	dst = ip6_route_output(net, NULL, &fl6);
 	if (!dst->error)
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH net-next 3/7] ipv6: Use ipv6_get_dsfield() instead of ipv6_tclass().
From: YOSHIFUJI Hideaki @ 2013-01-13 15:02 UTC (permalink / raw)
  To: davem, netdev; +Cc: yoshfuji

Commit 7a3198a8 ("ipv6: helper function to get tclass") introduced
ipv6_tclass(), but similar function is already available as
ipv6_get_dsfield().

We might be able to call ipv6_tclass() from ipv6_get_dsfield(),
but it is confusing to have two versions.

Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 include/linux/ipv6.h |    5 -----
 net/ipv6/datagram.c  |    3 ++-
 net/ipv6/tcp_ipv6.c  |    6 +++---
 3 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index faed1e3..304a9f4 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -77,11 +77,6 @@ static inline struct ipv6hdr *ipipv6_hdr(const struct sk_buff *skb)
 	return (struct ipv6hdr *)skb_transport_header(skb);
 }
 
-static inline __u8 ipv6_tclass(const struct ipv6hdr *iph)
-{
-	return (ntohl(*(__be32 *)iph) >> 20) & 0xff;
-}
-
 /* 
    This structure contains results of exthdrs parsing
    as offsets from skb->nh.
diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
index 7f65df4..33be363 100644
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -30,6 +30,7 @@
 #include <net/transp_v6.h>
 #include <net/ip6_route.h>
 #include <net/tcp_states.h>
+#include <net/dsfield.h>
 
 #include <linux/errqueue.h>
 #include <asm/uaccess.h>
@@ -487,7 +488,7 @@ int datagram_recv_ctl(struct sock *sk, struct msghdr *msg, struct sk_buff *skb)
 	}
 
 	if (np->rxopt.bits.rxtclass) {
-		int tclass = ipv6_tclass(ipv6_hdr(skb));
+		int tclass = ipv6_get_dsfield(ipv6_hdr(skb));
 		put_cmsg(msg, SOL_IPV6, IPV6_TCLASS, sizeof(tclass), &tclass);
 	}
 
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 3164ad2..3701c3c 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1163,7 +1163,7 @@ static struct sock * tcp_v6_syn_recv_sock(struct sock *sk, struct sk_buff *skb,
 		newnp->opt	   = NULL;
 		newnp->mcast_oif   = inet6_iif(skb);
 		newnp->mcast_hops  = ipv6_hdr(skb)->hop_limit;
-		newnp->rcv_tclass  = ipv6_tclass(ipv6_hdr(skb));
+		newnp->rcv_tclass  = ipv6_get_dsfield(ipv6_hdr(skb));
 
 		/*
 		 * No need to charge this sock to the relevant IPv6 refcnt debug socks count
@@ -1243,7 +1243,7 @@ static struct sock * tcp_v6_syn_recv_sock(struct sock *sk, struct sk_buff *skb,
 	newnp->opt	  = NULL;
 	newnp->mcast_oif  = inet6_iif(skb);
 	newnp->mcast_hops = ipv6_hdr(skb)->hop_limit;
-	newnp->rcv_tclass = ipv6_tclass(ipv6_hdr(skb));
+	newnp->rcv_tclass = ipv6_get_dsfield(ipv6_hdr(skb));
 
 	/* Clone native IPv6 options from listening socket (if any)
 
@@ -1456,7 +1456,7 @@ ipv6_pktoptions:
 		if (np->rxopt.bits.rxhlim || np->rxopt.bits.rxohlim)
 			np->mcast_hops = ipv6_hdr(opt_skb)->hop_limit;
 		if (np->rxopt.bits.rxtclass)
-			np->rcv_tclass = ipv6_tclass(ipv6_hdr(skb));
+			np->rcv_tclass = ipv6_get_dsfield(ipv6_hdr(skb));
 		if (ipv6_opt_accepted(sk, opt_skb)) {
 			skb_set_owner_r(opt_skb, sk);
 			opt_skb = xchg(&np->pktoptions, opt_skb);
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH net-next 4/7] ipv6: Make ipv6_is_mld() inline and use it from ip6_mc_input().
From: YOSHIFUJI Hideaki @ 2013-01-13 15:02 UTC (permalink / raw)
  To: davem, netdev; +Cc: yoshfuji

Move generalized version of ipv6_is_mld() to header,
and use it from ip6_mc_input().

Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 include/net/addrconf.h |   26 +++++++++++++++++++++++++-
 net/ipv6/ip6_input.c   |   21 +++------------------
 net/ipv6/mcast.c       |   27 ---------------------------
 3 files changed, 28 insertions(+), 46 deletions(-)

diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index df4ef94..7cd14c0 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -150,7 +150,31 @@ extern void addrconf_dad_failure(struct inet6_ifaddr *ifp);
 extern bool ipv6_chk_mcast_addr(struct net_device *dev,
 				const struct in6_addr *group,
 				const struct in6_addr *src_addr);
-extern bool ipv6_is_mld(struct sk_buff *skb, int nexthdr);
+
+/*
+ * identify MLD packets for MLD filter exceptions
+ */
+static inline bool ipv6_is_mld(struct sk_buff *skb, int nexthdr, int offset)
+{
+	struct icmp6hdr *hdr;
+
+	if (nexthdr != IPPROTO_ICMPV6 ||
+	    !pskb_network_may_pull(skb, offset + sizeof(struct icmp6hdr)))
+		return false;
+
+	hdr = (struct icmp6hdr *)(skb_network_header(skb) + offset);
+
+	switch (hdr->icmp6_type) {
+	case ICMPV6_MGM_QUERY:
+	case ICMPV6_MGM_REPORT:
+	case ICMPV6_MGM_REDUCTION:
+	case ICMPV6_MLD2_REPORT:
+		return true;
+	default:
+		break;
+	}
+	return false;
+}
 
 extern void addrconf_prefix_rcv(struct net_device *dev,
 				u8 *opt, int len, bool sllao);
diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
index a52d864..2ccd35e 100644
--- a/net/ipv6/ip6_input.c
+++ b/net/ipv6/ip6_input.c
@@ -212,7 +212,7 @@ resubmit:
 			if (ipv6_addr_is_multicast(&hdr->daddr) &&
 			    !ipv6_chk_mcast_addr(skb->dev, &hdr->daddr,
 			    &hdr->saddr) &&
-			    !ipv6_is_mld(skb, nexthdr))
+			    !ipv6_is_mld(skb, nexthdr, skb_network_header_len(skb)))
 				goto discard;
 		}
 		if (!(ipprot->flags & INET6_PROTO_NOPOLICY) &&
@@ -283,7 +283,6 @@ int ip6_mc_input(struct sk_buff *skb)
 		if (unlikely(opt->ra)) {
 			/* Check if this is a mld message */
 			u8 *ptr = skb_network_header(skb) + opt->ra;
-			struct icmp6hdr *icmp6;
 			u8 nexthdr = hdr->nexthdr;
 			__be16 frag_off;
 			int offset;
@@ -303,24 +302,10 @@ int ip6_mc_input(struct sk_buff *skb)
 				if (offset < 0)
 					goto out;
 
-				if (nexthdr != IPPROTO_ICMPV6)
+				if (!ipv6_is_mld(skb, nexthdr, offset))
 					goto out;
 
-				if (!pskb_may_pull(skb, (skb_network_header(skb) +
-						   offset + 1 - skb->data)))
-					goto out;
-
-				icmp6 = (struct icmp6hdr *)(skb_network_header(skb) + offset);
-
-				switch (icmp6->icmp6_type) {
-				case ICMPV6_MGM_QUERY:
-				case ICMPV6_MGM_REPORT:
-				case ICMPV6_MGM_REDUCTION:
-				case ICMPV6_MLD2_REPORT:
-					deliver = true;
-					break;
-				}
-				goto out;
+				deliver = true;
 			}
 			/* unknown RA - process it normally */
 		}
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 28dfa5f..8237ee1 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -935,33 +935,6 @@ int ipv6_dev_mc_dec(struct net_device *dev, const struct in6_addr *addr)
 }
 
 /*
- * identify MLD packets for MLD filter exceptions
- */
-bool ipv6_is_mld(struct sk_buff *skb, int nexthdr)
-{
-	struct icmp6hdr *pic;
-
-	if (nexthdr != IPPROTO_ICMPV6)
-		return false;
-
-	if (!pskb_may_pull(skb, sizeof(struct icmp6hdr)))
-		return false;
-
-	pic = icmp6_hdr(skb);
-
-	switch (pic->icmp6_type) {
-	case ICMPV6_MGM_QUERY:
-	case ICMPV6_MGM_REPORT:
-	case ICMPV6_MGM_REDUCTION:
-	case ICMPV6_MLD2_REPORT:
-		return true;
-	default:
-		break;
-	}
-	return false;
-}
-
-/*
  *	check if the interface/address pair is valid
  */
 bool ipv6_chk_mcast_addr(struct net_device *dev, const struct in6_addr *group,
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH net-next 5/7] ipv6 route: Use ipv6_addr_hash() in rt6_info_hash_nhsfn().
From: YOSHIFUJI Hideaki @ 2013-01-13 15:02 UTC (permalink / raw)
  To: davem, netdev; +Cc: yoshfuji

Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 net/ipv6/route.c |   11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 6238eb5..34f392f0 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -388,15 +388,8 @@ static int rt6_info_hash_nhsfn(unsigned int candidate_count,
 {
 	unsigned int val = fl6->flowi6_proto;
 
-	val ^= (__force u32)fl6->daddr.s6_addr32[0];
-	val ^= (__force u32)fl6->daddr.s6_addr32[1];
-	val ^= (__force u32)fl6->daddr.s6_addr32[2];
-	val ^= (__force u32)fl6->daddr.s6_addr32[3];
-
-	val ^= (__force u32)fl6->saddr.s6_addr32[0];
-	val ^= (__force u32)fl6->saddr.s6_addr32[1];
-	val ^= (__force u32)fl6->saddr.s6_addr32[2];
-	val ^= (__force u32)fl6->saddr.s6_addr32[3];
+	val ^= ipv6_addr_hash(&fl6->daddr);
+	val ^= ipv6_addr_hash(&fl6->saddr);
 
 	/* Work only if this not encapsulated */
 	switch (fl6->flowi6_proto) {
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH net-next 6/7] ipv6 xfrm: Use ipv6_addr_hash() in xfrm6_tunnel_spi_hash_byaddr().
From: YOSHIFUJI Hideaki @ 2013-01-13 15:02 UTC (permalink / raw)
  To: davem, netdev; +Cc: yoshfuji

Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 net/ipv6/xfrm6_tunnel.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv6/xfrm6_tunnel.c b/net/ipv6/xfrm6_tunnel.c
index ee5a706..babd167 100644
--- a/net/ipv6/xfrm6_tunnel.c
+++ b/net/ipv6/xfrm6_tunnel.c
@@ -72,7 +72,7 @@ static inline unsigned int xfrm6_tunnel_spi_hash_byaddr(const xfrm_address_t *ad
 {
 	unsigned int h;
 
-	h = (__force u32)(addr->a6[0] ^ addr->a6[1] ^ addr->a6[2] ^ addr->a6[3]);
+	h = ipv6_addr_hash((const struct in6_addr *)addr);
 	h ^= h >> 16;
 	h ^= h >> 8;
 	h &= XFRM6_TUNNEL_SPI_BYADDR_HSIZE - 1;
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH net-next 7/7] ipv6: Store Router Alert option in IP6CB directly.
From: YOSHIFUJI Hideaki @ 2013-01-13 15:02 UTC (permalink / raw)
  To: davem, netdev; +Cc: yoshfuji

Router Alert option is very small and we can store the value
itself in the skb.

Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 include/linux/ipv6.h      |    3 ++-
 include/uapi/linux/ipv6.h |    2 ++
 net/ipv6/exthdrs.c        |    3 ++-
 net/ipv6/ip6_input.c      |    5 ++---
 4 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index 304a9f4..e971e37 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -84,7 +84,7 @@ static inline struct ipv6hdr *ipipv6_hdr(const struct sk_buff *skb)
 
 struct inet6_skb_parm {
 	int			iif;
-	__u16			ra;
+	__be16			ra;
 	__u16			hop;
 	__u16			dst0;
 	__u16			srcrt;
@@ -100,6 +100,7 @@ struct inet6_skb_parm {
 #define IP6SKB_XFRM_TRANSFORMED	1
 #define IP6SKB_FORWARDED	2
 #define IP6SKB_REROUTED		4
+#define IP6SKB_ROUTERALERT	8
 };
 
 #define IP6CB(skb)	((struct inet6_skb_parm*)((skb)->cb))
diff --git a/include/uapi/linux/ipv6.h b/include/uapi/linux/ipv6.h
index 5a2991c..4bda4cf 100644
--- a/include/uapi/linux/ipv6.h
+++ b/include/uapi/linux/ipv6.h
@@ -63,6 +63,8 @@ struct ipv6_opt_hdr {
 #define ipv6_destopt_hdr ipv6_opt_hdr
 #define ipv6_hopopt_hdr  ipv6_opt_hdr
 
+/* Router Alert option values (RFC2711) */
+#define IPV6_OPT_ROUTERALERT_MLD	0x0000	/* MLD(RFC2710) */
 
 /*
  *	routing header type 0 (used in cmsghdr struct)
diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c
index 473f628..07a7d65 100644
--- a/net/ipv6/exthdrs.c
+++ b/net/ipv6/exthdrs.c
@@ -553,7 +553,8 @@ static bool ipv6_hop_ra(struct sk_buff *skb, int optoff)
 	const unsigned char *nh = skb_network_header(skb);
 
 	if (nh[optoff + 1] == 2) {
-		IP6CB(skb)->ra = optoff;
+		IP6CB(skb)->flags |= IP6SKB_ROUTERALERT;
+		memcpy(&IP6CB(skb)->ra, nh + optoff + 2, sizeof(IP6CB(skb)->ra));
 		return true;
 	}
 	LIMIT_NETDEBUG(KERN_DEBUG "ipv6_hop_ra: wrong RA length %d\n",
diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
index 2ccd35e..4ac5bf3 100644
--- a/net/ipv6/ip6_input.c
+++ b/net/ipv6/ip6_input.c
@@ -280,9 +280,8 @@ int ip6_mc_input(struct sk_buff *skb)
 		struct inet6_skb_parm *opt = IP6CB(skb);
 
 		/* Check for MLD */
-		if (unlikely(opt->ra)) {
+		if (unlikely(opt->flags & IP6SKB_ROUTERALERT)) {
 			/* Check if this is a mld message */
-			u8 *ptr = skb_network_header(skb) + opt->ra;
 			u8 nexthdr = hdr->nexthdr;
 			__be16 frag_off;
 			int offset;
@@ -290,7 +289,7 @@ int ip6_mc_input(struct sk_buff *skb)
 			/* Check if the value of Router Alert
 			 * is for MLD (0x0000).
 			 */
-			if ((ptr[2] | ptr[3]) == 0) {
+			if (opt->ra == htons(IPV6_OPT_ROUTERALERT_MLD)) {
 				deliver = false;
 
 				if (!ipv6_ext_hdr(nexthdr)) {
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH net-next 0/7] IPv6 updates
From: YOSHIFUJI Hideaki @ 2013-01-13 15:02 UTC (permalink / raw)
  To: davem, netdev; +Cc: yoshfuji

Hello.

YOSHIFUJI Hideaki (7):
  ipv6: Introduce ip6_flow_hdr() to fill version, tclass and flowlabel.
  ipv6: Introduce ip6_flowinfo() to extract flowinfo (tclass +
    flowlabel).
  ipv6: Use ipv6_get_dsfield() instead of ipv6_tclass().
  ipv6: Make ipv6_is_mld() inline and use it from ip6_mc_input().
  ipv6 route: Use ipv6_addr_hash() in rt6_info_hash_nhsfn().
  ipv6 xfrm: Use ipv6_addr_hash() in xfrm6_tunnel_spi_hash_byaddr().
  ipv6: Store Router Alert option in IP6CB directly.

 include/linux/ipv6.h             |    8 ++------
 include/net/addrconf.h           |   26 +++++++++++++++++++++++++-
 include/net/ipv6.h               |   14 ++++++++++++++
 include/uapi/linux/ipv6.h        |    2 ++
 net/ipv6/datagram.c              |   12 +++++++-----
 net/ipv6/exthdrs.c               |    3 ++-
 net/ipv6/ip6_gre.c               |    6 ++----
 net/ipv6/ip6_input.c             |   26 +++++---------------------
 net/ipv6/ip6_output.c            |    8 +++-----
 net/ipv6/ip6_tunnel.c            |    4 +---
 net/ipv6/mcast.c                 |   27 ---------------------------
 net/ipv6/netfilter/ip6t_REJECT.c |    2 +-
 net/ipv6/route.c                 |   17 +++++------------
 net/ipv6/tcp_ipv6.c              |    6 +++---
 net/ipv6/xfrm6_tunnel.c          |    2 +-
 15 files changed, 73 insertions(+), 90 deletions(-)

-- 
1.7.9.5

^ permalink raw reply

* Re: [PATCH 01/19] netfilter: move nf_conntrack initialize out of pernet operations
From: Pablo Neira Ayuso @ 2013-01-13 15:07 UTC (permalink / raw)
  To: Gao feng; +Cc: netfilter-devel, netdev, canqunzhang, kaber, ebiederm
In-Reply-To: <50EF6470.5000303@cn.fujitsu.com>

On Fri, Jan 11, 2013 at 09:01:36AM +0800, Gao feng wrote:
> On 2013/01/11 00:41, Pablo Neira Ayuso wrote:
> > First off, thanks for looking into this.
> > 
> > I want to get this fix into 3.8 and -stable but this patch includes a
> > rework whose scope is net-next (upcoming 3.9).
> > 
> > The attached patch aims to fix the issue according to your patch
> > description. Once this is in, we can revisit your code refactoring
> > proposal.
> > 
> > Let me know.
> > 
> 
> Yes,I'm happy this bug being fixed in 3.8.
> So what I should do is waiting for below patch being accepted and
> then rebase my patchset? It's OK.

Yes, you'll have to rebase upon nf-next. First, I have to ask David to
pull from nf to get this fixes, so you'll have to wait a bit. If any
doubt, contact me.

While at it, please, merge patches 09, 10 and 11 in one single patch.

And 12 to 18. We need that the repository has to remain consistent
between two patches.

No problem if the patch seems big as soon as changes belong to the
same scope.

> Thanks!
[...]
> Acked-by: Gao feng <gaofeng@cn.fujitsu.com>

And this patch applied, thanks a lot Gao.

^ permalink raw reply

* Re: what is the mail list address of network users ?
From: Xose Vazquez Perez @ 2013-01-13 15:59 UTC (permalink / raw)
  To: horserivers, netdev

horseriver wrote:

> not develop

Linux Advanced Routing & Traffic Control

http://lartc.org
http://vger.kernel.org/vger-lists.html#lartc

^ 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