Netdev List
 help / color / mirror / Atom feed
* [RFC(v2) net-next 00/13] IPv6 rt->n removal (V2)
From: YOSHIFUJI Hideaki @ 2013-01-15 16:44 UTC (permalink / raw)
  To: netdev; +Cc: yoshfuji, xiyou.wangcong

Hello.

This is RFC (Take 2).

Some unused variable / code still remain.
Need to revisit refcounting (rt->dst.dev, rt->n->dev).

--yoshfuji

YOSHIFUJI Hideaki (13):
  ndisc: Refer/Update neigh->updated with write lock.
  ndisc: Remove tbl argument for __ipv6_neigh_lookup().
  ndisc: Introduce __ipv6_neigh_lookup_noref().
  ipv6 route: Dump gateway based on RTF_GATEWAY flag and
    rt->rt6i_gateway.
  ipv6: Introduce rt6_nexthop() to select nexthop address.
  ipv6: Do not depend on rt->n in ip6_finish_output2().
  ipv6: Do not repend on rt->n in rt6_probe().
  ipv6: Do not deoend on rt->n in ip6_dst_lookup_tail().
  ipv6: Do not depend on rt->n in rt6_check_neigh().
  ipv6: Do not depend on rt->n in ip6_pol_route().
  ipv6: Use ip6_route_output() instead of icmp6_dst_alloc().
  ipv6: Remove temporary dst for icmpv6.
  ipv6: Complete neighbour entry removal from dst_entry.

 include/net/ip6_fib.h   |    2 -
 include/net/ip6_route.h |    9 ++
 include/net/ndisc.h     |   24 +++--
 net/ipv6/ip6_fib.c      |    2 +-
 net/ipv6/ip6_output.c   |   26 ++++--
 net/ipv6/mcast.c        |    5 +-
 net/ipv6/ndisc.c        |    3 +-
 net/ipv6/route.c        |  222 ++++++++---------------------------------------
 net/ipv6/xfrm6_policy.c |    1 -
 9 files changed, 86 insertions(+), 208 deletions(-)

-- 
1.7.9.5

^ permalink raw reply

* Re: [PATCH 1/5] soreuseport: infrastructure
From: Eric Dumazet @ 2013-01-15 16:14 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Tom Herbert, netdev, davem, netdev
In-Reply-To: <20130115075323.64b2fdc7@nehalam.linuxnetplumber.net>

On Tue, 2013-01-15 at 07:53 -0800, Stephen Hemminger wrote:
> On Mon, 14 Jan 2013 12:00:18 -0800 (PST)
> Tom Herbert <therbert@google.com> wrote:
> 
> > +/* Pseudo random number generator from numerical recipes. */
> > +static inline u32 next_pseudo_random32(u32 seed)
> > +{
> > +	return seed * 1664525 + 1013904223;
> > +}
> > +
> 
> Don't reimplement a pseudo random number generator, there already
> exists net_random() 

net_random() is way more expensive and not needed in this context.

If you have 32 listeners bound on the same port, we can call this 32
times per SYN message.

Initial seed is random enough (phash = inet_ehashfn(net, daddr,
hnum,saddr,sport)

Anyway, the full idea of distributing SYN using a random generator is
not the best one for a multi queue NIC, and/or if RPS/RFS is used.

Ideally, we should chose a target given by the current CPU number, in
case SYN messages are spread on all cpus or a set of cpus.

(same idea than PACKET_FANOUT_CPU in net/packet/af_packet.c)

^ permalink raw reply

* Re: [PATCH 3/5] soreuseport: UDP/IPv4 implementation
From: Stephen Hemminger @ 2013-01-15 15:58 UTC (permalink / raw)
  To: Tom Herbert; +Cc: netdev, davem, netdev, eric.dumazet
In-Reply-To: <alpine.DEB.2.00.1301141140320.3523@pokey.mtv.corp.google.com>

On Mon, 14 Jan 2013 12:00:23 -0800 (PST)
Tom Herbert <therbert@google.com> wrote:

> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index 79c8dbe..1dd1e93 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -139,6 +139,7 @@ static int udp_lib_lport_inuse(struct net *net, __u16 num,
>  {
>  	struct sock *sk2;
>  	struct hlist_nulls_node *node;
> +	int uid = sock_i_uid(sk);
>  

I am not a namespace expert, but it looks like this might have
to use from_kuid_munged(). At a minimum it should be:
         kuid_t uid = sock_i_uid(sk);

^ permalink raw reply

* Re: [PATCH 1/5] soreuseport: infrastructure
From: Stephen Hemminger @ 2013-01-15 15:53 UTC (permalink / raw)
  To: Tom Herbert; +Cc: netdev, davem, netdev, eric.dumazet
In-Reply-To: <alpine.DEB.2.00.1301141152130.3433@pokey.mtv.corp.google.com>

On Mon, 14 Jan 2013 12:00:18 -0800 (PST)
Tom Herbert <therbert@google.com> wrote:

> +/* Pseudo random number generator from numerical recipes. */
> +static inline u32 next_pseudo_random32(u32 seed)
> +{
> +	return seed * 1664525 + 1013904223;
> +}
> +

Don't reimplement a pseudo random number generator, there already
exists net_random() 

^ permalink raw reply

* [PATCH] cxgb4: set coalesce parameters on all queues
From: Thadeu Lima de Souza Cascardo @ 2013-01-15 15:15 UTC (permalink / raw)
  To: netdev; +Cc: dm, linux-kernel, Thadeu Lima de Souza Cascardo, steved, toml

The coalesce parameters was set only on the first queue, which caused
interrupt rates to be larger on all the other queues.

This patch allows interrupt rates to be reduced for certain workloads
and colaesce parameters by 41%.

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
Cc: steved@us.ibm.com
Cc: toml@us.ibm.com
---
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c |   17 ++++++++++++++---
 1 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index 0df1284..53ec2ad 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -1762,9 +1762,20 @@ static int set_coalesce(struct net_device *dev, struct ethtool_coalesce *c)
 {
 	const struct port_info *pi = netdev_priv(dev);
 	struct adapter *adap = pi->adapter;
-
-	return set_rxq_intr_params(adap, &adap->sge.ethrxq[pi->first_qset].rspq,
-			c->rx_coalesce_usecs, c->rx_max_coalesced_frames);
+	struct sge_rspq *q;
+	int i;
+	int r = 0;
+
+	for (i = pi->first_qset; i < pi->first_qset + pi->nqsets; i++) {
+		q = &adap->sge.ethrxq[i].rspq;
+		r = set_rxq_intr_params(adap, q, c->rx_coalesce_usecs,
+			c->rx_max_coalesced_frames);
+		if (r) {
+			dev_err(&dev->dev, "failed to set coalesce %d\n", r);
+			break;
+		}
+	}
+	return r;
 }
 
 static int get_coalesce(struct net_device *dev, struct ethtool_coalesce *c)
-- 
1.7.1

^ permalink raw reply related

* Re: [PATCH net-next 09/10] bnx2x: Added FW GRO bridging support
From: Eric Dumazet @ 2013-01-15 15:07 UTC (permalink / raw)
  To: Yuval Mintz; +Cc: davem, netdev, eilong, ariele
In-Reply-To: <50F56178.8000502@broadcom.com>

On Tue, 2013-01-15 at 16:02 +0200, Yuval Mintz wrote:
> >>> bnx2x not setting gso_segs means that qdisc accounting on ingress is
> >>> completely wrong.
> >>
> >> Notice this behaviour was not introduced in this patch -
> >>
> >>	...
> >>
> >> We could either:
> >>   1. Continue with this patch, later sending a patch correcting gso_segs,
> >>      as this is not a new issue.
> >>   2. Send a V2 patch-series which will also set gso_segs correctly.
> > 
> > I am fine with any solution, as long as we fix the problem.
> 
> Eric - Thanks.
> Just to be certain - gso_segs should hold the number of non-aggregated packets
> contained in the skb's frags, right?

Thats right

If the FW doesnt provide the info, you could for example do :

shinfo->gso_segs = DIV_ROUND_UP(skb->len - hdr_len, shinfo->gso_size);

(hdr_len being the length of all headers (ethernet+network+TCP))

^ permalink raw reply

* Re: [RFC PATCH v3] cgroup: net_cls: traffic counter based on classification control cgroup
From: Eric Dumazet @ 2013-01-15 15:05 UTC (permalink / raw)
  To: Alexey Perevalov
  Cc: cgroups-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20130115133351.GA8633@aperevalov>

On Tue, 2013-01-15 at 17:33 +0400, Alexey Perevalov wrote:
> Hello
> 
> I would like to represent next version of patch I sent before
> cgroup: "net_cls: traffic counter based on classification control cgroup"
> 
> The main idea is the same as was. It keeping counter in control groups, but now uses atomic instead of resource_counters.
> 
> Signed-off-by: Alexey Perevalov <a.perevalov-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>  include/net/cls_cgroup.h         |  200 ++++++++++++++++++++++++++++++++++----
>  include/net/cls_counter_holder.h |   26 +++++
>  init/Kconfig                     |   25 +++++
>  kernel/cgroup.c                  |    2 +
>  kernel/res_counter.c             |    4 +
>  net/core/dev.c                   |    6 ++
>  net/ipv4/tcp.c                   |   27 ++++-
>  net/ipv4/udp.c                   |    6 ++
>  net/sched/Kconfig                |   11 ---
>  net/sched/Makefile               |    1 +
>  net/sched/cls_cgroup.c           |  194 +++++++++++++++++++++++++++++++++++-
>  net/sched/cls_counter_holder.c   |  144 +++++++++++++++++++++++++++
>  12 files changed, 611 insertions(+), 35 deletions(-)
>  create mode 100644 include/net/cls_counter_holder.h
>  create mode 100644 net/sched/cls_counter_holder.c

Sorry, there are too many issues with this patch and I have not
a lot of time to review it.

0) No changelog or documentation

1) There is no way you need to add kludges in tcp and udp for this
accounting. Retransmitted packets should not be ignored for example.

2) Ugly #ifdef in c files. Don't do that please.

3) No check of kmalloc() returns (can be NULL)

^ permalink raw reply

* Re: [PATCH net-next 09/10] bnx2x: Added FW GRO bridging support
From: Yuval Mintz @ 2013-01-15 14:02 UTC (permalink / raw)
  To: Eric Dumazet, davem; +Cc: netdev, eilong, ariele
In-Reply-To: <1358260752.8744.5677.camel@edumazet-glaptop>

>>> bnx2x not setting gso_segs means that qdisc accounting on ingress is
>>> completely wrong.
>>
>> Notice this behaviour was not introduced in this patch -
>>
>>	...
>>
>> We could either:
>>   1. Continue with this patch, later sending a patch correcting gso_segs,
>>      as this is not a new issue.
>>   2. Send a V2 patch-series which will also set gso_segs correctly.
> 
> I am fine with any solution, as long as we fix the problem.

Eric - Thanks.
Just to be certain - gso_segs should hold the number of non-aggregated packets
contained in the skb's frags, right?


Dave -

Considering Eric's response, following with option (1) or (2) seems like the
right way to go.

Do you want a new patch series which will include a fix for this,
or will you accept a later fix that sets the gso_segs correctly?

(considering this issue is not introduced in this patch,
merely isn't being solved by it)

Thanks,
Yuval Mintz

^ permalink raw reply

* Re: [PATCH net-next 09/10] bnx2x: Added FW GRO bridging support
From: Eric Dumazet @ 2013-01-15 14:39 UTC (permalink / raw)
  To: Yuval Mintz; +Cc: davem, netdev, eilong, ariele
In-Reply-To: <50F50537.3030608@broadcom.com>

On Tue, 2013-01-15 at 09:28 +0200, Yuval Mintz wrote:
> On 01/14/2013 08:44 PM, Eric Dumazet wrote:
> > On Mon, 2013-01-14 at 09:22 -0800, Eric Dumazet wrote:
> > 
> > What is the value of gso_segs ?
> > 
> > The reason I am pointing this out is the recent change in commit
> > 1def9238d4aa2146924994aa4b7dc861f03b9362
> > (net_sched: more precise pkt_len computation)
> > 
> > bnx2x not setting gso_segs means that qdisc accounting on ingress is
> > completely wrong.
> 
> Hi Eric,
> 
> First I just want to state that you're totally correct about the gso_segs -
> bnx2x is not setting it correctly (it's currently totally omitted), and so
> it would incorrectly affect the accounting.
> 
> However, notice this behaviour was not introduced in this patch -
> Since 621b4d6 bnx2x is using FW GRO, overriding the kernel's GRO implementation:
> As the bnx2x driver is supplied with the aggregated packet from its FW,
> the bnx2x passes a value in the `gso_size' field of its skb, causing
> `skb_is_gso' to return `true'.
> This will cause the aggregated skb to override the GRO machinations
> (in `dev_gro_receive'), overriding all calls to `gro_receive' and thus also
> the call to `skb_gro_receive' which whould have incremented `gso_segs'.
> 
> This patch actually tries to correct said behaviour, obviously failing
> with the gso_segs but still improving the current state of bnx2x GRO
> in bridging scenarios.
> 
> >> This looks weird to me. This should be called by GRO stack only.
> 
> I think this is the main question - we could try and implement this
> inside the network-core itself, but as said behaviour is unique to the
> bnx2x driver (correct me if I'm wrong, but I'm unaware of any other
> driver which does GRO without the kernel GRO implementation), the
> solution is specially tailored for the bnx2x driver.
> 
> We could either:
>   1. Continue with this patch, later sending a patch correcting gso_segs,
>      as this is not a new issue.
>   2. Send a V2 patch-series which will also set gso_segs correctly.
>   3. Send a V2 patch-series which omits this patch, and later send an RFC
>      for some kernel implementation which fixes the issue.
> 
> Your thoughts on this matter will be greatly appreciated.

I am fine with any solution, as long as we fix the problem.

If GRO is done by the FW/driver instead of core network stack, we should
make sure :

- transport_header is set correctly (your patch seems to do that)
- gso_segs is computed (this could be done in core network, but this
adds yet another conditional test in th fast path, and it seems only
bnx2x would need this)

Thanks

^ permalink raw reply

* [RFC patch net-next] ipv4: introduce address lifetime
From: Jiri Pirko @ 2013-01-15 13:43 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, shemminger, kuznet, jmorris, yoshfuji, kaber,
	psimerda, tgraf, nicolas.dichtel, nhorman, dcbw

There are some usecase when lifetime of ipv4 addresses might be helpful.
For example:
1) initramfs networkmanager uses a DHCP daemon to learn network
configuration parameters
2) initramfs networkmanager addresses, routes and DNS configuration
3) initramfs networkmanager is requested to stop
4) initramfs networkmanager stops all daemons including dhclient
5) there are addresses and routes configured but no daemon running. If
the system doesn't start networkmanager for some reason, addresses and
routes will be used forever, which violates RFC 2131.

This patch is essentially a backport of ivp6 address lifetime mechanism
for ipv4 addresses.

Current "ip" tool supports this without any patch (since it does not
distinguish between ipv4 and ipv6 addresses in this perspective.

Also, this should be back-compatible with all current netlink users.

I did not took care of "modify" process yet. I would like to hear your
opinions if this is acceptable first.

Reported-by: Pavel Šimerda <psimerda@redhat.com>
Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 include/linux/inetdevice.h |   6 ++
 include/net/addrconf.h     |   4 ++
 net/ipv4/devinet.c         | 172 ++++++++++++++++++++++++++++++++++++++++++++-
 net/ipv6/addrconf.c        |   4 --
 4 files changed, 180 insertions(+), 6 deletions(-)

diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
index a9d8289..ea1e3b8 100644
--- a/include/linux/inetdevice.h
+++ b/include/linux/inetdevice.h
@@ -166,6 +166,12 @@ struct in_ifaddr {
 	unsigned char		ifa_flags;
 	unsigned char		ifa_prefixlen;
 	char			ifa_label[IFNAMSIZ];
+
+	/* In seconds, relative to tstamp. Expiry is at tstamp + HZ * lft. */
+	__u32			ifa_valid_lft;
+	__u32			ifa_preferred_lft;
+	unsigned long		ifa_cstamp; /* created timestamp */
+	unsigned long		ifa_tstamp; /* updated timestamp */
 };
 
 extern int register_inetaddr_notifier(struct notifier_block *nb);
diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index 7cd14c0..0aca314 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -15,6 +15,10 @@
 
 #define IPV6_MAX_ADDRESSES		16
 
+#define ADDRCONF_TIMER_FUZZ_MINUS	(HZ > 50 ? HZ / 50 : 1)
+#define ADDRCONF_TIMER_FUZZ		(HZ / 4)
+#define ADDRCONF_TIMER_FUZZ_MAX		(HZ)
+
 #include <linux/in.h>
 #include <linux/in6.h>
 
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index cc06a47..9a9773e 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -63,6 +63,7 @@
 #include <net/ip_fib.h>
 #include <net/rtnetlink.h>
 #include <net/net_namespace.h>
+#include <net/addrconf.h>
 
 #include "fib_lookup.h"
 
@@ -93,6 +94,7 @@ static const struct nla_policy ifa_ipv4_policy[IFA_MAX+1] = {
 	[IFA_ADDRESS]   	= { .type = NLA_U32 },
 	[IFA_BROADCAST] 	= { .type = NLA_U32 },
 	[IFA_LABEL]     	= { .type = NLA_STRING, .len = IFNAMSIZ - 1 },
+	[IFA_CACHEINFO]		= { .len = sizeof(struct ifa_cacheinfo) },
 };
 
 #define IN4_ADDR_HSIZE_SHIFT	8
@@ -417,6 +419,10 @@ static void inet_del_ifa(struct in_device *in_dev, struct in_ifaddr **ifap,
 	__inet_del_ifa(in_dev, ifap, destroy, NULL, 0);
 }
 
+static void check_lifetime(struct work_struct *work);
+
+static DECLARE_DELAYED_WORK(check_lifetime_work, check_lifetime);
+
 static int __inet_insert_ifa(struct in_ifaddr *ifa, struct nlmsghdr *nlh,
 			     u32 portid)
 {
@@ -462,6 +468,9 @@ static int __inet_insert_ifa(struct in_ifaddr *ifa, struct nlmsghdr *nlh,
 
 	inet_hash_insert(dev_net(in_dev->dev), ifa);
 
+	cancel_delayed_work(&check_lifetime_work);
+	schedule_delayed_work(&check_lifetime_work, 0);
+
 	/* Send message first, then call notifier.
 	   Notifier will trigger FIB update, so that
 	   listeners of netlink will know about new ifaddr */
@@ -573,6 +582,105 @@ errout:
 	return err;
 }
 
+#define INFINITY_LIFE_TIME	0xFFFFFFFF
+
+static void check_lifetime(struct work_struct *work)
+{
+	unsigned long now, next, next_sec, next_sched;
+	struct in_ifaddr *ifa;
+	struct hlist_node *node;
+	int i;
+
+	now = jiffies;
+	next = round_jiffies_up(now + ADDR_CHECK_FREQUENCY);
+
+	rcu_read_lock();
+	for (i = 0; i < IN4_ADDR_HSIZE; i++) {
+		hlist_for_each_entry_rcu(ifa, node,
+					 &inet_addr_lst[i], hash) {
+			unsigned long age;
+
+			if (ifa->ifa_flags & IFA_F_PERMANENT)
+				continue;
+
+			/* We try to batch several events at once. */
+			age = (now - ifa->ifa_tstamp +
+			       ADDRCONF_TIMER_FUZZ_MINUS) / HZ;
+
+			if (ifa->ifa_valid_lft != INFINITY_LIFE_TIME &&
+			    age >= ifa->ifa_valid_lft) {
+				struct in_ifaddr **ifap ;
+
+				rtnl_lock();
+				for (ifap = &ifa->ifa_dev->ifa_list;
+				     *ifap != NULL; ifap = &ifa->ifa_next) {
+					if (*ifap == ifa)
+						inet_del_ifa(ifa->ifa_dev,
+							     ifap, 1);
+				}
+				rtnl_unlock();
+			} else if (ifa->ifa_preferred_lft ==
+				   INFINITY_LIFE_TIME) {
+				continue;
+			} else if (age >= ifa->ifa_preferred_lft) {
+				if (time_before(ifa->ifa_tstamp +
+						ifa->ifa_valid_lft * HZ, next))
+					next = ifa->ifa_tstamp +
+					       ifa->ifa_valid_lft * HZ;
+
+				if (!(ifa->ifa_flags & IFA_F_DEPRECATED)) {
+					ifa->ifa_flags |= IFA_F_DEPRECATED;
+					rtmsg_ifa(RTM_NEWADDR, ifa, NULL, 0);
+				}
+			} else if (time_before(ifa->ifa_tstamp +
+					       ifa->ifa_preferred_lft * HZ,
+					       next)) {
+				next = ifa->ifa_tstamp +
+				       ifa->ifa_preferred_lft * HZ;
+			}
+		}
+	}
+	rcu_read_unlock();
+
+	next_sec = round_jiffies_up(next);
+	next_sched = next;
+
+	/* If rounded timeout is accurate enough, accept it. */
+	if (time_before(next_sec, next + ADDRCONF_TIMER_FUZZ))
+		next_sched = next_sec;
+
+	now = jiffies;
+	/* And minimum interval is ADDRCONF_TIMER_FUZZ_MAX. */
+	if (time_before(next_sched, now + ADDRCONF_TIMER_FUZZ_MAX))
+		next_sched = now + ADDRCONF_TIMER_FUZZ_MAX;
+
+	schedule_delayed_work(&check_lifetime_work, next_sched - now);
+}
+
+static void set_ifa_lifetime(struct in_ifaddr *ifa, __u32 valid_lft,
+			     __u32 prefered_lft)
+{
+	unsigned long timeout;
+
+	ifa->ifa_flags &= ~(IFA_F_PERMANENT | IFA_F_DEPRECATED);
+
+	timeout = addrconf_timeout_fixup(valid_lft, HZ);
+	if (addrconf_finite_timeout(timeout))
+		ifa->ifa_valid_lft = timeout;
+	else
+		ifa->ifa_flags |= IFA_F_PERMANENT;
+
+	timeout = addrconf_timeout_fixup(prefered_lft, HZ);
+	if (addrconf_finite_timeout(timeout)) {
+		if (timeout == 0)
+			ifa->ifa_flags |= IFA_F_DEPRECATED;
+		ifa->ifa_preferred_lft = timeout;
+	}
+	ifa->ifa_tstamp = jiffies;
+	if (!ifa->ifa_cstamp)
+		ifa->ifa_cstamp = ifa->ifa_tstamp;
+}
+
 static struct in_ifaddr *rtm_to_ifaddr(struct net *net, struct nlmsghdr *nlh)
 {
 	struct nlattr *tb[IFA_MAX+1];
@@ -633,6 +741,19 @@ static struct in_ifaddr *rtm_to_ifaddr(struct net *net, struct nlmsghdr *nlh)
 	else
 		memcpy(ifa->ifa_label, dev->name, IFNAMSIZ);
 
+	if (tb[IFA_CACHEINFO]) {
+		struct ifa_cacheinfo *ci;
+
+		ci = nla_data(tb[IFA_CACHEINFO]);
+		if (!ci->ifa_valid || ci->ifa_prefered > ci->ifa_valid) {
+			err = -EINVAL;
+			goto errout;
+		}
+		set_ifa_lifetime(ifa, ci->ifa_valid, ci->ifa_prefered);
+	} else {
+		set_ifa_lifetime(ifa, INFINITY_LIFE_TIME, INFINITY_LIFE_TIME);
+	}
+
 	return ifa;
 
 errout:
@@ -852,6 +973,7 @@ int devinet_ioctl(struct net *net, unsigned int cmd, void __user *arg)
 			ifa->ifa_prefixlen = 32;
 			ifa->ifa_mask = inet_make_mask(32);
 		}
+		set_ifa_lifetime(ifa, INFINITY_LIFE_TIME, INFINITY_LIFE_TIME);
 		ret = inet_set_ifa(dev, ifa);
 		break;
 
@@ -1190,6 +1312,8 @@ static int inetdev_event(struct notifier_block *this, unsigned long event,
 				ifa->ifa_dev = in_dev;
 				ifa->ifa_scope = RT_SCOPE_HOST;
 				memcpy(ifa->ifa_label, dev->name, IFNAMSIZ);
+				set_ifa_lifetime(ifa, INFINITY_LIFE_TIME,
+						 INFINITY_LIFE_TIME);
 				inet_insert_ifa(ifa);
 			}
 		}
@@ -1246,11 +1370,30 @@ static size_t inet_nlmsg_size(void)
 	       + nla_total_size(IFNAMSIZ); /* IFA_LABEL */
 }
 
+static inline u32 cstamp_delta(unsigned long cstamp)
+{
+	return (cstamp - INITIAL_JIFFIES) * 100UL / HZ;
+}
+
+static int put_cacheinfo(struct sk_buff *skb, unsigned long cstamp,
+			 unsigned long tstamp, u32 preferred, u32 valid)
+{
+	struct ifa_cacheinfo ci;
+
+	ci.cstamp = cstamp_delta(cstamp);
+	ci.tstamp = cstamp_delta(tstamp);
+	ci.ifa_prefered = preferred;
+	ci.ifa_valid = valid;
+
+	return nla_put(skb, IFA_CACHEINFO, sizeof(ci), &ci);
+}
+
 static int inet_fill_ifaddr(struct sk_buff *skb, struct in_ifaddr *ifa,
 			    u32 portid, u32 seq, int event, unsigned int flags)
 {
 	struct ifaddrmsg *ifm;
 	struct nlmsghdr  *nlh;
+	u32 preferred, valid;
 
 	nlh = nlmsg_put(skb, portid, seq, event, sizeof(*ifm), flags);
 	if (nlh == NULL)
@@ -1259,10 +1402,31 @@ static int inet_fill_ifaddr(struct sk_buff *skb, struct in_ifaddr *ifa,
 	ifm = nlmsg_data(nlh);
 	ifm->ifa_family = AF_INET;
 	ifm->ifa_prefixlen = ifa->ifa_prefixlen;
-	ifm->ifa_flags = ifa->ifa_flags|IFA_F_PERMANENT;
+	ifm->ifa_flags = ifa->ifa_flags;
 	ifm->ifa_scope = ifa->ifa_scope;
 	ifm->ifa_index = ifa->ifa_dev->dev->ifindex;
 
+	if (!(ifm->ifa_flags & IFA_F_PERMANENT)) {
+		preferred = ifa->ifa_preferred_lft;
+		valid = ifa->ifa_valid_lft;
+		if (preferred != INFINITY_LIFE_TIME) {
+			long tval = (jiffies - ifa->ifa_tstamp) / HZ;
+
+			if (preferred > tval)
+				preferred -= tval;
+			else
+				preferred = 0;
+			if (valid != INFINITY_LIFE_TIME) {
+				if (valid > tval)
+					valid -= tval;
+				else
+					valid = 0;
+			}
+		}
+	} else {
+		preferred = INFINITY_LIFE_TIME;
+		valid = INFINITY_LIFE_TIME;
+	}
 	if ((ifa->ifa_address &&
 	     nla_put_be32(skb, IFA_ADDRESS, ifa->ifa_address)) ||
 	    (ifa->ifa_local &&
@@ -1270,7 +1434,9 @@ static int inet_fill_ifaddr(struct sk_buff *skb, struct in_ifaddr *ifa,
 	    (ifa->ifa_broadcast &&
 	     nla_put_be32(skb, IFA_BROADCAST, ifa->ifa_broadcast)) ||
 	    (ifa->ifa_label[0] &&
-	     nla_put_string(skb, IFA_LABEL, ifa->ifa_label)))
+	     nla_put_string(skb, IFA_LABEL, ifa->ifa_label)) ||
+	    put_cacheinfo(skb, ifa->ifa_cstamp, ifa->ifa_tstamp,
+			  preferred, valid))
 		goto nla_put_failure;
 
 	return nlmsg_end(skb, nlh);
@@ -1988,6 +2154,8 @@ void __init devinet_init(void)
 	register_gifconf(PF_INET, inet_gifconf);
 	register_netdevice_notifier(&ip_netdev_notifier);
 
+	schedule_delayed_work(&check_lifetime_work, 0);
+
 	rtnl_af_register(&inet_af_ops);
 
 	rtnl_register(PF_INET, RTM_NEWADDR, inet_rtm_newaddr, NULL, NULL);
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 408cac4a..64c3ed6 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -110,10 +110,6 @@ static inline u32 cstamp_delta(unsigned long cstamp)
 	return (cstamp - INITIAL_JIFFIES) * 100UL / HZ;
 }
 
-#define ADDRCONF_TIMER_FUZZ_MINUS	(HZ > 50 ? HZ/50 : 1)
-#define ADDRCONF_TIMER_FUZZ		(HZ / 4)
-#define ADDRCONF_TIMER_FUZZ_MAX		(HZ)
-
 #ifdef CONFIG_SYSCTL
 static void addrconf_sysctl_register(struct inet6_dev *idev);
 static void addrconf_sysctl_unregister(struct inet6_dev *idev);
-- 
1.8.1

^ permalink raw reply related

* [RFC PATCH v3] cgroup: net_cls: traffic counter based on classification control cgroup
From: Alexey Perevalov @ 2013-01-15 13:33 UTC (permalink / raw)
  To: cgroups-u79uwXL29TY76Z2rM5mHXA; +Cc: netdev-u79uwXL29TY76Z2rM5mHXA

Hello

I would like to represent next version of patch I sent before
cgroup: "net_cls: traffic counter based on classification control cgroup"

The main idea is the same as was. It keeping counter in control groups, but now uses atomic instead of resource_counters.

Signed-off-by: Alexey Perevalov <a.perevalov-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 include/net/cls_cgroup.h         |  200 ++++++++++++++++++++++++++++++++++----
 include/net/cls_counter_holder.h |   26 +++++
 init/Kconfig                     |   25 +++++
 kernel/cgroup.c                  |    2 +
 kernel/res_counter.c             |    4 +
 net/core/dev.c                   |    6 ++
 net/ipv4/tcp.c                   |   27 ++++-
 net/ipv4/udp.c                   |    6 ++
 net/sched/Kconfig                |   11 ---
 net/sched/Makefile               |    1 +
 net/sched/cls_cgroup.c           |  194 +++++++++++++++++++++++++++++++++++-
 net/sched/cls_counter_holder.c   |  144 +++++++++++++++++++++++++++
 12 files changed, 611 insertions(+), 35 deletions(-)
 create mode 100644 include/net/cls_counter_holder.h
 create mode 100644 net/sched/cls_counter_holder.c

diff --git a/include/net/cls_cgroup.h b/include/net/cls_cgroup.h
index 2581638..e3bfe6f 100644
--- a/include/net/cls_cgroup.h
+++ b/include/net/cls_cgroup.h
@@ -13,54 +13,197 @@
 #ifndef _NET_CLS_CGROUP_H
 #define _NET_CLS_CGROUP_H
 
+#include <linux/atomic.h>
 #include <linux/cgroup.h>
 #include <linux/hardirq.h>
 #include <linux/rcupdate.h>
 
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+#include <linux/nsproxy.h>
+#include <linux/string.h>
+#include <linux/skbuff.h>
+#include <net/cls_counter_holder.h>
+#include <net/sock.h>
+
+/*TODO hide all it to separate file*/
+
+struct cls_iface_cntrs {
+	char *dev_name;
+	atomic64_t snd_counter;
+	atomic64_t rcv_counter;
+	struct list_head link;
+};
+
+#endif /*CONFIG_NET_CLS_COUNTER*/
+
+
 #if IS_ENABLED(CONFIG_NET_CLS_CGROUP)
 struct cgroup_cls_state
 {
 	struct cgroup_subsys_state css;
 	u32 classid;
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+	struct cls_iface_cntrs iface_stats;
+#endif /*CONFIG_NET_CLS_COUNTER*/
 };
 
 extern void sock_update_classid(struct sock *sk, struct task_struct *task);
 
-#if IS_BUILTIN(CONFIG_NET_CLS_CGROUP)
-static inline u32 task_cls_classid(struct task_struct *p)
+#if IS_MODULE(CONFIG_NET_CLS_CGROUP)
+static inline struct cgroup_cls_state *get_cls_cgroup(struct task_struct *p)
 {
-	u32 classid;
+	struct cgroup_subsys_state *css = task_subsys_state(p,
+		net_cls_subsys_id);
+	if (css)
+		return container_of(css,
+				       struct cgroup_cls_state, css);
+	return NULL;
+}
+#elif IS_BUILTIN(CONFIG_NET_CLS_CGROUP)
+static inline struct cgroup_cls_state *get_cls_cgroup(struct task_struct *p)
+{
+	return container_of(task_subsys_state(p, net_cls_subsys_id),
+			       struct cgroup_cls_state, css);
+}
+#endif
 
-	if (in_interrupt())
-		return 0;
 
-	rcu_read_lock();
-	classid = container_of(task_subsys_state(p, net_cls_subsys_id),
-			       struct cgroup_cls_state, css)->classid;
-	rcu_read_unlock();
+#endif /*CONFIG_NET_CLS_CGROUP*/
 
-	return classid;
+#if IS_ENABLED(CONFIG_NET_CLS_CGROUP)
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+static inline u32 skb_cls_classid(const struct sk_buff *skb)
+{
+	return (skb && skb->sk) ? skb->sk->sk_classid : 0;
+}
+
+static inline int get_ifindex_from_skb(const struct sk_buff *skb)
+{
+	int ifindex = 0;
+	if (skb)
+		ifindex = skb->skb_iif;
+	return ifindex;
+}
+
+static struct cls_iface_cntrs *find_cls_counter(
+	struct cgroup_cls_state *cls_cgroup,
+	const char *dev_name,
+	bool create)
+{
+	/*TODO Add lock*/
+	struct cls_iface_cntrs *entry = NULL;
+
+	if (!dev_name) {
+		pr_err("cls please provide valid dev name");
+		return NULL;
+	}
+
+	list_for_each_entry(entry, &cls_cgroup->iface_stats.link, link)
+		if (!strcmp(entry->dev_name, dev_name))
+			return entry;
+
+	if (!create)
+		return entry;
+
+	/*not found, insert*/
+	entry = kmalloc(sizeof(struct cls_iface_cntrs), GFP_ATOMIC);
+	entry->dev_name = kstrdup(dev_name, GFP_ATOMIC);
+	atomic64_set(&entry->rcv_counter, 0);
+	atomic64_set(&entry->snd_counter, 0);
+	list_add_tail(&entry->link, &cls_cgroup->iface_stats.link);
+	return entry;
 }
-#elif IS_MODULE(CONFIG_NET_CLS_CGROUP)
+
+static void charge_net_cls_snd(struct cgroup_cls_state *cls_cgroup,
+	const u32 copied, const char *dev_name)
+{
+	struct cls_iface_cntrs *cnt = find_cls_counter(cls_cgroup,
+		dev_name, true);
+
+	if (!cnt)
+		return;
+
+	atomic64_add(copied, &cnt->snd_counter);
+}
+
+static char *get_dev_name(const int ifindex)
+{
+	struct net *net = NULL;
+	struct nsproxy *nsproxy = NULL;
+	struct net_device *net_dev = NULL;
+
+	nsproxy = task_nsproxy(current);
+	if (!nsproxy) {
+		pr_debug("cls cant find task_nsproxy");
+		return NULL;
+	}
+
+	net = get_net(nsproxy->net_ns);
+	if (!net) {
+		pr_debug("cls cant find net");
+		return NULL;
+	}
+	net_dev = dev_get_by_index(net, ifindex);
+
+	return net_dev ? net_dev->name : NULL;
+}
+
+static void charge_net_cls_rcv(struct cgroup_cls_state *cls_cgroup,
+	const u32 copied, const int ifindex)
+{
+	char *dev_name = get_dev_name(ifindex);
+	struct cls_iface_cntrs *cnt = find_cls_counter(cls_cgroup,
+		dev_name, true);
+
+	if (!cnt)
+		return;
+
+	atomic64_add(copied, &cnt->rcv_counter);
+}
+
+static inline void count_cls_rcv(struct task_struct *p, const u32 copied,
+	const int ifindex)
+{
+	struct cgroup_cls_state *cls_cgroup;
+
+	cls_cgroup = get_cls_cgroup(p);
+
+	if (cls_cgroup)
+		charge_net_cls_rcv(cls_cgroup, copied, ifindex);
+}
+
+static inline void count_cls_snd(u32 classid, const u32 copied,
+	const char *dev_name)
+{
+	struct cgroup_cls_state *cls_cgroup;
+
+	cls_cgroup = find_cls_cgroup_by_classid(classid);
+
+	if (cls_cgroup)
+		charge_net_cls_snd(cls_cgroup, copied, dev_name);
+}
+#endif /*CONFIG_NET_CLS_COUNTER*/
+
 static inline u32 task_cls_classid(struct task_struct *p)
 {
-	struct cgroup_subsys_state *css;
-	u32 classid = 0;
+	int classid = 0;
+	struct cgroup_cls_state *cls_cgroup = NULL;
 
 	if (in_interrupt())
 		return 0;
 
 	rcu_read_lock();
-	css = task_subsys_state(p, net_cls_subsys_id);
-	if (css)
-		classid = container_of(css,
-				       struct cgroup_cls_state, css)->classid;
+
+	cls_cgroup = get_cls_cgroup(p);
+	if (cls_cgroup)
+		classid = cls_cgroup->classid;
+
 	rcu_read_unlock();
 
 	return classid;
 }
-#endif
-#else /* !CGROUP_NET_CLS_CGROUP */
+
+#else /* !CONFIG_NET_CLS_CGROUP */
 static inline void sock_update_classid(struct sock *sk, struct task_struct *task)
 {
 }
@@ -69,5 +212,22 @@ static inline u32 task_cls_classid(struct task_struct *p)
 {
 	return 0;
 }
-#endif /* CGROUP_NET_CLS_CGROUP */
+#endif /* CONFIG_NET_CLS_CGROUP */
+
+#if !IS_ENABLED(CONFIG_NET_CLS_CGROUP) || !IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+static inline void count_cls_rcv(struct task_struct *p, const u32 copied,
+				 const int ifindex)
+{
+}
+
+static inline void count_cls_snd(u32 classid, const u32 copied,
+				 const char *dev_name)
+{
+}
+
+static inline u32 skb_cls_classid(const struct sk_buff *skb)
+{
+	return 0;
+}
+#endif
 #endif  /* _NET_CLS_CGROUP_H */
diff --git a/include/net/cls_counter_holder.h b/include/net/cls_counter_holder.h
new file mode 100644
index 0000000..a129baa
--- /dev/null
+++ b/include/net/cls_counter_holder.h
@@ -0,0 +1,26 @@
+/*
+ * cls_counter_holder.c  Interface for holding references of the
+ *                       net cls cgroup instances.
+ *
+ * Authors:	Alexey Perevalov, <a.perevalov-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
+ *
+ * Changes:
+ *		This program is free software; you can redistribute it and/or
+ *		modify it under the terms of the GNU General Public License
+ *		as published by the Free Software Foundation; either version
+ *		2 of the License, or (at your option) any later version.
+ */
+
+#ifndef _NET_CLS_COUNTER_HOLDER_H_
+#define _NET_CLS_COUNTER_HOLDER_H_
+
+#include <net/cls_cgroup.h>
+
+struct cgroup_cls_state;
+
+void insert_cls_cgroup_entry(struct cgroup_cls_state *obj);
+void delete_cls_cgroup_entry(const u32 classid);
+struct cgroup_cls_state *find_cls_cgroup_by_classid(const u32 classid);
+
+
+#endif /* _NET_CLS_COUNTER_HOLDER_H_ */
diff --git a/init/Kconfig b/init/Kconfig
index 7d30240..6e01fc2 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -906,6 +906,31 @@ config CGROUP_HUGETLB
 	  control group is tracked in the third page lru pointer. This means
 	  that we cannot use the controller with huge page less than 3 pages.
 
+menuconfig NET_CLS_CGROUP
+	tristate "Control Group Classifier"
+	select NET_CLS
+	depends on CGROUPS
+	---help---
+	  Say Y here if you want to classify packets based on the control
+	  cgroup of their process.
+
+	  To compile this code as a module, choose M here: the
+	  module will be called cls_cgroup.
+
+if NET_CLS_CGROUP
+config NET_CLS_COUNTER
+	bool "Network traffic counter for network Control Group Classifier"
+	select NET_CLS
+	default n
+	depends on NET_CLS_CGROUP && RESOURCE_COUNTERS
+	---help---
+	  Say Y here if you want to count traffic associate with the control
+	  cgroup.
+
+	  To add functionality to cls_cgroup select y.
+
+endif #NET_CLS_CGROUP
+
 config CGROUP_PERF
 	bool "Enable perf_event per-cpu per-container group (cgroup) monitoring"
 	depends on PERF_EVENTS && CGROUPS
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 4855892..cd82a9e 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -2897,6 +2897,8 @@ int cgroup_rm_cftypes(struct cgroup_subsys *ss, struct cftype *cfts)
 	cgroup_cfts_commit(ss, NULL, false);
 	return -ENOENT;
 }
+EXPORT_SYMBOL_GPL(cgroup_rm_cftypes);
+
 
 /**
  * cgroup_task_count - count the number of tasks in a cgroup.
diff --git a/kernel/res_counter.c b/kernel/res_counter.c
index ff55247..a51b501 100644
--- a/kernel/res_counter.c
+++ b/kernel/res_counter.c
@@ -13,6 +13,8 @@
 #include <linux/res_counter.h>
 #include <linux/uaccess.h>
 #include <linux/mm.h>
+#include <linux/export.h>
+
 
 void res_counter_init(struct res_counter *counter, struct res_counter *parent)
 {
@@ -21,6 +23,7 @@ void res_counter_init(struct res_counter *counter, struct res_counter *parent)
 	counter->soft_limit = RESOURCE_MAX;
 	counter->parent = parent;
 }
+EXPORT_SYMBOL(res_counter_init);
 
 int res_counter_charge_locked(struct res_counter *counter, unsigned long val,
 			      bool force)
@@ -176,6 +179,7 @@ u64 res_counter_read_u64(struct res_counter *counter, int member)
 	return *res_counter_member(counter, member);
 }
 #endif
+EXPORT_SYMBOL(res_counter_read_u64);
 
 int res_counter_memparse_write_strategy(const char *buf,
 					unsigned long long *res)
diff --git a/net/core/dev.c b/net/core/dev.c
index d1e8116..ffc9ec2 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -135,6 +135,7 @@
 #include <linux/net_tstamp.h>
 #include <linux/static_key.h>
 #include <net/flow_keys.h>
+#include <net/cls_cgroup.h>
 
 #include "net-sysfs.h"
 
@@ -2922,6 +2923,11 @@ int dev_queue_xmit(struct sk_buff *skb)
 	 */
 	rcu_read_lock_bh();
 
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+	if (dev)
+		count_cls_snd(skb_cls_classid(skb), skb->len, dev->name);
+#endif
+
 	skb_update_prio(skb);
 
 	txq = netdev_pick_tx(dev, skb);
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 1ca2536..dc4dc3a 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -276,6 +276,7 @@
 #include <net/ip.h>
 #include <net/netdma.h>
 #include <net/sock.h>
+#include <net/cls_cgroup.h>
 
 #include <asm/uaccess.h>
 #include <asm/ioctls.h>
@@ -1464,6 +1465,9 @@ int tcp_read_sock(struct sock *sk, read_descriptor_t *desc,
 	u32 seq = tp->copied_seq;
 	u32 offset;
 	int copied = 0;
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+	int ifindex = 0;
+#endif
 
 	if (sk->sk_state == TCP_LISTEN)
 		return -ENOTCONN;
@@ -1510,6 +1514,9 @@ int tcp_read_sock(struct sock *sk, read_descriptor_t *desc,
 			++seq;
 			break;
 		}
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+		ifindex = get_ifindex_from_skb(skb);
+#endif
 		sk_eat_skb(sk, skb, false);
 		if (!desc->count)
 			break;
@@ -1520,8 +1527,12 @@ int tcp_read_sock(struct sock *sk, read_descriptor_t *desc,
 	tcp_rcv_space_adjust(sk);
 
 	/* Clean up data we have read: This will do ACK frames. */
-	if (copied > 0)
+	if (copied > 0) {
 		tcp_cleanup_rbuf(sk, copied);
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+		count_cls_rcv(current, copied, ifindex);
+#endif
+	}
 	return copied;
 }
 EXPORT_SYMBOL(tcp_read_sock);
@@ -1549,6 +1560,9 @@ int tcp_recvmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
 	bool copied_early = false;
 	struct sk_buff *skb;
 	u32 urg_hole = 0;
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+	int ifindex = 0;
+#endif
 
 	lock_sock(sk);
 
@@ -1873,6 +1887,9 @@ skip_copy:
 		if (tcp_hdr(skb)->fin)
 			goto found_fin_ok;
 		if (!(flags & MSG_PEEK)) {
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+			ifindex = get_ifindex_from_skb(skb);
+#endif
 			sk_eat_skb(sk, skb, copied_early);
 			copied_early = false;
 		}
@@ -1882,6 +1899,9 @@ skip_copy:
 		/* Process the FIN. */
 		++*seq;
 		if (!(flags & MSG_PEEK)) {
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+			ifindex = get_ifindex_from_skb(skb);
+#endif
 			sk_eat_skb(sk, skb, copied_early);
 			copied_early = false;
 		}
@@ -1924,6 +1944,11 @@ skip_copy:
 	/* Clean up data we have read: This will do ACK frames. */
 	tcp_cleanup_rbuf(sk, copied);
 
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+	if (copied > 0)
+		count_cls_rcv(current, copied, ifindex);
+#endif
+
 	release_sock(sk);
 	return copied;
 
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 79c8dbe..a143629 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -101,6 +101,7 @@
 #include <linux/skbuff.h>
 #include <linux/proc_fs.h>
 #include <linux/seq_file.h>
+#include <net/cls_cgroup.h>
 #include <net/net_namespace.h>
 #include <net/icmp.h>
 #include <net/route.h>
@@ -1254,6 +1255,11 @@ try_again:
 	if (flags & MSG_TRUNC)
 		err = ulen;
 
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+	if (ulen > 0)
+		count_cls_rcv(current, ulen, get_ifindex_from_skb(skb));
+#endif
+
 out_free:
 	skb_free_datagram_locked(sk, skb);
 out:
diff --git a/net/sched/Kconfig b/net/sched/Kconfig
index 235e01a..ac7bcdb 100644
--- a/net/sched/Kconfig
+++ b/net/sched/Kconfig
@@ -418,17 +418,6 @@ config NET_CLS_FLOW
 	  To compile this code as a module, choose M here: the
 	  module will be called cls_flow.
 
-config NET_CLS_CGROUP
-	tristate "Control Group Classifier"
-	select NET_CLS
-	depends on CGROUPS
-	---help---
-	  Say Y here if you want to classify packets based on the control
-	  cgroup of their process.
-
-	  To compile this code as a module, choose M here: the
-	  module will be called cls_cgroup.
-
 config NET_EMATCH
 	bool "Extended Matches"
 	select NET_CLS
diff --git a/net/sched/Makefile b/net/sched/Makefile
index 978cbf0..95dbb12 100644
--- a/net/sched/Makefile
+++ b/net/sched/Makefile
@@ -49,6 +49,7 @@ obj-$(CONFIG_NET_CLS_RSVP6)	+= cls_rsvp6.o
 obj-$(CONFIG_NET_CLS_BASIC)	+= cls_basic.o
 obj-$(CONFIG_NET_CLS_FLOW)	+= cls_flow.o
 obj-$(CONFIG_NET_CLS_CGROUP)	+= cls_cgroup.o
+obj-$(CONFIG_NET_CLS_COUNTER)   += cls_counter_holder.o
 obj-$(CONFIG_NET_EMATCH)	+= ematch.o
 obj-$(CONFIG_NET_EMATCH_CMP)	+= em_cmp.o
 obj-$(CONFIG_NET_EMATCH_NBYTE)	+= em_nbyte.o
diff --git a/net/sched/cls_cgroup.c b/net/sched/cls_cgroup.c
index 3a294eb..1535a97 100644
--- a/net/sched/cls_cgroup.c
+++ b/net/sched/cls_cgroup.c
@@ -9,6 +9,7 @@
  * Authors:	Thomas Graf <tgraf-G/eBtMaohhA@public.gmane.org>
  */
 
+#include <linux/atomic.h>
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/types.h>
@@ -22,6 +23,15 @@
 #include <net/pkt_cls.h>
 #include <net/sock.h>
 #include <net/cls_cgroup.h>
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+#include <linux/rbtree.h>
+#include <net/cls_counter_holder.h>
+
+static struct notifier_block counter_notifier;
+static const char *rcv_label = "rcv:";
+static const char *snd_label = "snd:";
+
+#endif
 
 static inline struct cgroup_cls_state *cgrp_cls_state(struct cgroup *cgrp)
 {
@@ -42,9 +52,38 @@ static struct cgroup_subsys_state *cgrp_css_alloc(struct cgroup *cgrp)
 	cs = kzalloc(sizeof(*cs), GFP_KERNEL);
 	if (!cs)
 		return ERR_PTR(-ENOMEM);
+
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+	atomic64_set(&cs->iface_stats.snd_counter, 0);
+	atomic64_set(&cs->iface_stats.rcv_counter, 0);
+	cs->iface_stats.dev_name = 0;
+	INIT_LIST_HEAD(&cs->iface_stats.link);
+#endif
+
 	return &cs->css;
 }
 
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+static inline void cgrp_counter_destroy(struct cgroup_cls_state *cs)
+{
+	struct list_head *pos, *q;
+	delete_cls_cgroup_entry(cs->classid);
+
+	list_for_each_safe(pos, q, &cs->iface_stats.link) {
+		struct cls_iface_cntrs *tmp = list_entry(
+			pos, struct cls_iface_cntrs, link);
+		list_del(pos);
+		if (!tmp)
+			continue;
+
+		if (!tmp->dev_name)
+			kfree(tmp->dev_name);
+		kfree(tmp);
+	}
+
+}
+#endif
+
 static int cgrp_css_online(struct cgroup *cgrp)
 {
 	if (cgrp->parent)
@@ -55,6 +94,14 @@ static int cgrp_css_online(struct cgroup *cgrp)
 
 static void cgrp_css_free(struct cgroup *cgrp)
 {
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+
+	struct cgroup_cls_state *cs = cgrp_cls_state(cgrp);
+
+	if (!cs)
+		return;
+	cgrp_counter_destroy(cs);
+#endif
 	kfree(cgrp_cls_state(cgrp));
 }
 
@@ -85,9 +132,57 @@ static u64 read_classid(struct cgroup *cgrp, struct cftype *cft)
 	return cgrp_cls_state(cgrp)->classid;
 }
 
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+static const char *extract_dev_name(const char *cgroup_file_name)
+{
+	const char *dot = strchr(cgroup_file_name, '.');
+	const size_t len = dot ?
+			dot - cgroup_file_name : strlen(cgroup_file_name);
+
+	return kstrndup(cgroup_file_name, len, GFP_KERNEL);
+}
+
+static int read_stat(struct cgroup *cgrp, struct cftype *cft,
+		struct cgroup_map_cb *cb)
+{
+	struct cgroup_cls_state *cs = cgrp_cls_state(cgrp);
+	const char *dev_name = extract_dev_name(cft->name);
+	struct cls_iface_cntrs *res = find_cls_counter(cs, dev_name, false);
+
+	if (!res) {
+		pr_debug("cls cant read for cls");
+		return -EINVAL;
+	}
+
+	cb->fill(cb, rcv_label,
+		atomic64_read(&res->rcv_counter));
+	cb->fill(cb, snd_label,
+		atomic64_read(&res->snd_counter));
+
+	kfree(dev_name);
+	return 0;
+}
+#endif /*CONFIG_NET_CLS_COUNTER*/
+
 static int write_classid(struct cgroup *cgrp, struct cftype *cft, u64 value)
 {
-	cgrp_cls_state(cgrp)->classid = (u32) value;
+	struct cgroup_cls_state *cgrp_cls = cgrp_cls_state(cgrp);
+	u32 *classid = &cgrp_cls->classid;
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+	u32 oldclassid = *classid;
+
+	if (find_cls_cgroup_by_classid(value)) {
+		pr_err("cls: classid %llu already exists\n", value);
+		return -EINVAL;
+	}
+
+	insert_cls_cgroup_entry(cgrp_cls);
+
+	if (oldclassid)
+		delete_cls_cgroup_entry(oldclassid);
+#endif /*CONFIG_NET_CLS_COUNTER*/
+	*classid = (u32) value;
+
 	return 0;
 }
 
@@ -304,17 +399,107 @@ static struct tcf_proto_ops cls_cgroup_ops __read_mostly = {
 	.owner		=	THIS_MODULE,
 };
 
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+static inline int add_cft_file_for_device(struct net_device *dev)
+{
+	struct cftype *cft;
+	int ret = 0;
+
+	if (!dev)
+		return ret;
+
+	cft = kmalloc(sizeof(struct cftype) * 2,
+		GFP_KERNEL);
+	/* *2 and last 0 fill for terminator */
+	memset(cft, 0, sizeof(struct cftype) * 2);
+
+	snprintf(cft->name, MAX_CFTYPE_NAME,
+		"%s.usage_in_bytes", dev->name);
+	cft->read_map = read_stat;
+	cft->private = RES_USAGE;
+	ret = cgroup_add_cftypes(&net_cls_subsys, cft);
+	if (ret)
+		pr_err("cls error adding cft for counting at cls_cgroup %d\n",
+		       ret);
+	return ret;
+}
+
+static int device_state_cb(struct notifier_block *nb,
+	unsigned long state, void *arg)
+{
+	struct net_device *net = (struct net_device *)arg;
+	if (!nb || !net) {
+		pr_err("Not valid arguments for net_device notifier cb\n");
+		return 0;
+	}
+
+	if (state == NETDEV_REGISTER) {
+		pr_info("cls New device %s\n", net->name);
+		return add_cft_file_for_device(net);
+	}
+	return 0;
+}
+
+static inline int init_cgroup_counter(void)
+{
+	int ret = 0;
+	struct net_device *dev;
+	counter_notifier.notifier_call = device_state_cb;
+
+	ret = register_netdevice_notifier(&counter_notifier);
+	if (ret)
+		pr_err("cls Cant register nofier\n");
+
+	for_each_netdev(&init_net, dev) {
+		ret = add_cft_file_for_device(dev);
+		if (ret)
+			goto unregister_notifier;
+	}
+
+	return ret;
+unregister_notifier:
+
+	unregister_netdevice_notifier(&counter_notifier);
+	return ret;
+}
+
+static void release_cft(void)
+{
+	struct list_head *pos, *q;
+	list_for_each_safe(pos, q, &net_cls_subsys.cftsets) {
+		struct cftype_set *set =
+				list_entry(pos, struct cftype_set, node);
+		int ret = cgroup_rm_cftypes(&net_cls_subsys, set->cfts);
+		if (!ret) {
+			pr_err("cls cant remove cftypes\n");
+			break;
+		}
+
+		kfree(set->cfts);
+	}
+}
+#endif
+
 static int __init init_cgroup_cls(void)
 {
 	int ret;
-
 	ret = cgroup_load_subsys(&net_cls_subsys);
 	if (ret)
 		goto out;
 
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+	ret = init_cgroup_counter();
+	if (ret)
+		goto unload;
+#endif
+
 	ret = register_tcf_proto_ops(&cls_cgroup_ops);
 	if (ret)
-		cgroup_unload_subsys(&net_cls_subsys);
+		goto unload;
+
+	return 0;
+unload:
+	cgroup_unload_subsys(&net_cls_subsys);
 
 out:
 	return ret;
@@ -324,6 +509,9 @@ static void __exit exit_cgroup_cls(void)
 {
 	unregister_tcf_proto_ops(&cls_cgroup_ops);
 
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+	release_cft();
+#endif
 	cgroup_unload_subsys(&net_cls_subsys);
 }
 
diff --git a/net/sched/cls_counter_holder.c b/net/sched/cls_counter_holder.c
new file mode 100644
index 0000000..94ab285
--- /dev/null
+++ b/net/sched/cls_counter_holder.c
@@ -0,0 +1,144 @@
+/*
+ * net/sched/cls_counter_holder.c Interface for holding references of the
+ *                     net cls cgroup instances.
+ *
+ *		This program is free software; you can redistribute it and/or
+ *		modify it under the terms of the GNU General Public License
+ *		as published by the Free Software Foundation; either version
+ *		2 of the License, or (at your option) any later version.
+ *
+ * Authors:	Alexey Perevalov <a.perevalov-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
+ */
+
+
+#include <linux/export.h>
+#include <linux/module.h>
+#include <net/cls_cgroup.h>
+#include <net/cls_counter_holder.h>
+
+static struct rb_root classid_tree = RB_ROOT;
+static DEFINE_SPINLOCK(classid_tree_lock);
+
+struct entry {
+	struct cgroup_cls_state *data;
+	struct rb_node node;
+};
+
+static struct entry *find_entry(struct rb_root *root, const u32 classid)
+{
+	struct rb_node *node = root->rb_node;
+
+	while (node) {
+		struct entry *cls_entry = rb_entry(node, struct entry, node);
+		int result = 0;
+		if (!cls_entry || !cls_entry->data)
+			break;
+		result = cls_entry->data->classid - classid;
+
+		if (result < 0)
+			node = node->rb_left;
+		else if (result > 0)
+			node = node->rb_right;
+		else
+			return cls_entry;
+	}
+	return NULL;
+}
+
+void insert_cls_cgroup_entry(struct cgroup_cls_state *obj)
+{
+	struct rb_node **new;
+	struct rb_node *parent = NULL;
+	struct entry *new_entry;
+	unsigned long irq_flags = 0;
+
+	struct rb_root *root = &classid_tree;
+
+	spin_lock_irqsave(&classid_tree_lock, irq_flags);
+
+	new = &root->rb_node;
+
+	while (*new) {
+		struct entry *this = rb_entry(*new, struct entry, node);
+		/* Sort by classid, then by ifindex */
+		int result =
+			(this->data->classid - obj->classid);
+		parent = *new;
+		if (result < 0)
+			new = &((*new)->rb_left);
+		else if (result > 0)
+			new = &((*new)->rb_right);
+		else
+			goto unlock;
+	}
+
+	/* If we here, we need to insert new entry into tree */
+	new_entry = kmalloc(sizeof(struct entry), GFP_ATOMIC);
+	if (!new_entry)
+		goto unlock;
+
+	new_entry->data = obj;
+	/* Add new node and rebalance tree */
+	rb_link_node(&new_entry->node, parent, new);
+	rb_insert_color(&new_entry->node, root);
+
+unlock:
+	spin_unlock_irqrestore(&classid_tree_lock, irq_flags);
+}
+EXPORT_SYMBOL(insert_cls_cgroup_entry);
+
+void delete_cls_cgroup_entry(const u32 classid)
+{
+	unsigned long irq_flags = 0;
+	struct entry *data = NULL;
+	struct rb_root *root = &classid_tree;
+	spin_lock_irqsave(&classid_tree_lock, irq_flags);
+
+	data = find_entry(root, classid);
+
+	if (data) {
+		rb_erase(&data->node, root);
+		kfree(data);
+	}
+	spin_unlock_irqrestore(&classid_tree_lock, irq_flags);
+}
+EXPORT_SYMBOL(delete_cls_cgroup_entry);
+
+static void free_node(struct rb_node *root)
+{
+	struct entry *cur_entry = rb_entry(root, struct entry, node);
+	if (root->rb_left)
+		free_node(root->rb_left);
+	if (root->rb_right)
+		free_node(root->rb_right);
+	kfree(cur_entry);
+}
+
+static void free_classid_tree(void)
+{
+	unsigned long irq_flags = 0;
+
+	spin_lock_irqsave(&classid_tree_lock, irq_flags);
+
+	free_node(classid_tree.rb_node);
+
+	spin_unlock_irqrestore(&classid_tree_lock, irq_flags);
+}
+
+struct cgroup_cls_state *find_cls_cgroup_by_classid(const u32 classid)
+{
+	struct entry *cls_entry = find_entry(&classid_tree, classid);
+	if (cls_entry)
+		return cls_entry->data;
+
+	return NULL;
+}
+EXPORT_SYMBOL(find_cls_cgroup_by_classid);
+
+static void __exit exit_cls_counter_holder(void)
+{
+	free_classid_tree();
+}
+
+module_exit(exit_cls_counter_holder);
+MODULE_LICENSE("GPL");
-- 
1.7.9.5

^ permalink raw reply related

* Re: what is the use of poll function int a net_device struct
From: Rami Rosen @ 2013-01-15 13:20 UTC (permalink / raw)
  To: horseriver; +Cc: netdev
In-Reply-To: <20130114142136.GA17094@debian.localdomain>

Hi,

On Mon, Jan 14, 2013 at 4:21 PM, horseriver <horserivers@gmail.com> wrote:
> Where is the handler function defined  for an netcard's  IRQ

look for the request_irq() method; you register the handler with this method.

for example,
in e1000e:

http://lxr.free-electrons.com/source/drivers/net/ethernet/intel/e1000e/netdev.c

err = request_irq(adapter->msix_entries[vector].vector,
                           e1000_intr_msix_rx, 0,
                          adapter->rx_ring->name,
                             netdev);

so the handler is   e1000_intr_msix_rx().

When an IRQ has reached,  how to wake_up the wait_queue of the socket?
TCP/UDP packet goes up from the driver to L3 and then to L4 (in case
it is for this machine). In L4 you should find the
wake_up of the wait_queue of the socket.

Regards,
Rami Rosen
http://ramirose.wix.com/ramirosen

^ permalink raw reply

* Re: [PATCH] CDC_NCM adding support IFF_NOARP for infineon modem platform
From: Wei Shuai @ 2013-01-15 13:11 UTC (permalink / raw)
  To: Dan Williams
  Cc: David Miller, Greg Kroah-Hartman, Alexey Orishko, Bjørn Mork,
	linux-usb@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <1358183953.1713.28.camel@dcbw.foobar.com>

yes. usbnet has FLAG_POINTTOPOINT, but that's nothing to do with
IFF_POINTTOPOINT. At least we should do something to handle
relationship of these flags rather than only have different names.
or new flag FLAG_NOARP could be introduced to corresponding to IFF_NOARP.

2013/1/15 Dan Williams <dcbw@redhat.com>:
> On Sat, 2013-01-12 at 15:35 -0800, David Miller wrote:
>> From: Wei Shuai <cpuwolf@gmail.com>
>> Date: Sat, 12 Jan 2013 19:34:39 +0800
>>
>> > Infineon(now Intel) HSPA Modem platform NCM cannot support ARP. so I
>> > introduce a flag CDC_NCM_DRIVER_DATA_NOARP which is defined in
>> > driver_info:data. so later on, if more such buggy devices are found,
>> > they could use same flag to handle.
>>
>> Is it no able to do ARP or, the more likely case, does broadcast
>> not work at all?
>>
>> If it's the latter, IFF_NOARP is just making over the real problem.
>>
>> I'm not applying this, no hardware device should set IFF_NOARP.
>> You probably really want IFF_POINTOPOINT or similar.
>
> IFF_NOARP is already done for other WWAN devices (sierra_net, hso,
> cdc-ether, cdc-phonet, lg-vl600, etc) so there is some precedent.  Some
> drivers (phonet, hso) set *both* POINTTOPOINT and NOARP.  Is that
> redundant, and should all WWAN drivers be moved to only POINTTOPOINT?
>
> (aside: usbnet has FLAG_POINTTOPOINT, but that's nothing to do with
> IFF_POINTTOPOINT, it only controls whether the interface is named usbX
> or ethX.  Confusing.)
>
> Dan
>

^ permalink raw reply

* Re: [RFC Patch net-next] ipv6: look up neighbours on demand in ip6_finish_output2()
From: YOSHIFUJI Hideaki @ 2013-01-15 12:22 UTC (permalink / raw)
  To: Cong Wang; +Cc: David Miller, netdev, roland, Eric Dumazet, YOSHIFUJI Hideaki
In-Reply-To: <50F4BEB2.2050201@gmail.com>

Cong Wang wrote:
> On 01/15/2013 02:30 AM, David Miller wrote:
>>
>> This is a very incomplete patch.
>>
>> If this change were so simple, it would have been done by someone
>> else a long time ago.
>>
>> You must, in addition to the incredibly obvious changes in the packet
>> output path, completely eliminate the caching of the neighbour entry
>> in the ipv6 routes themselves.
>>
>> This means replacing every rt6->n access or test with something
>> equivalent.
>>
> 
> THanks, David and Eric!
> 
> I knew this is probably incomplete, that is why I added "RFC". Fortunately, YOSHIFUJI just sent a more complete patch:
> 
> [RFC net-next] ipv6 route: Do not attach neighbour on route.
> 
> So, please ignore mine and use his. :)

Well, in fact, it did't work.  Tt really needs further work.

--yoshfuji

^ permalink raw reply

* Re: the patch "bridge: export multicast database via netlink" broke kernel 3.8 uapi
From: Thomas Backlund @ 2013-01-15 10:55 UTC (permalink / raw)
  To: Cong Wang
  Cc: Thomas Backlund, Eric Blake, netdev, linux-kernel, libvirt-list,
	tgraf
In-Reply-To: <1358244688.4264.7.camel@cr0>

Cong Wang skrev 15.1.2013 12:11:
> On Tue, 2013-01-15 at 12:03 +0200, Thomas Backlund wrote:
>> Eric Blake skrev 15.1.2013 01:57:
>>> On 01/13/2013 01:05 PM, Thomas Backlund wrote:
>>>> Thomas Backlund skrev 13.1.2013 20:38:
>>>>> patch both inline and attached as thunderbird tends to mess up ...
>>>>
>>>>> -----
>>>>>
>>>>> if_bridge.h uses struct in6_addr ip6; but does not include the in6.h
>>>>> header.
>>>>>
>>>>> Found by trying to build libvirt and connman against 3.8-rc3 headers.
>>>>>
>>>>
>>>> Ok,
>>>> ignore this patch, it's not the correct fix as it introduces
>>>> redefinitions...
>>>>
>>>> Btw, the error that I hit that made me suggest this fix was libvirt
>>>> config check bailing out:
>>>>
>>>> config.log:/usr/include/linux/if_bridge.h:173:20: error: field 'ip6' has
>>>> incomplete type
>>>
>>> Hmm, just now noticing this thread, after independently hitting and and
>>> having to work around the same problem in libvirt:
>>> https://www.redhat.com/archives/libvir-list/2013-January/msg00930.html
>>> https://bugzilla.redhat.com/show_bug.cgi?id=895141
>>
>>
>> Yep,
>>
>> and the commit breaking uapi headers is by using "struct in6_addr ip6" is:
>>
>>
>>   From ee07c6e7a6f8a25c18f0a6b18152fbd7499245f6 Mon Sep 17 00:00:00 2001
>> From: Cong Wang <amwang@redhat.com>
>> Date: Fri, 7 Dec 2012 00:04:48 +0000
>> Subject: [PATCH] bridge: export multicast database via netlink
>
> Does the following patch help?
>
> $ git diff include/uapi/linux/if_bridge.h
> diff --git a/include/uapi/linux/if_bridge.h
> b/include/uapi/linux/if_bridge.h
> index 5db2975..653db23 100644
> --- a/include/uapi/linux/if_bridge.h
> +++ b/include/uapi/linux/if_bridge.h
> @@ -14,6 +14,7 @@
>   #define _UAPI_LINUX_IF_BRIDGE_H
>
>   #include <linux/types.h>
> +#include <linux/in6.h>
>
>   #define SYSFS_BRIDGE_ATTR      "bridge"
>   #define SYSFS_BRIDGE_FDB       "brforward"
>

Well, I suggested the same fix in the beginning of the thread
on netdev and lkml: "if_bridge.h: include in6.h for struct in6_addr use"

as it seemed to fix the libvirt case

but then asked it to be ignored after I tried to build connman,
and hit this conflict with glibc-2.17:

In file included from /usr/include/arpa/inet.h:22:0,
                  from ./include/connman/inet.h:25,
                  from src/connman.h:128,
                  from src/tethering.c:40:
/usr/include/netinet/in.h:35:5: error: expected identifier before 
numeric constant
/usr/include/netinet/in.h:197:8: error: redefinition of 'struct in6_addr'
In file included from /usr/include/linux/if_bridge.h:17:0,
                  from src/tethering.c:38:
/usr/include/linux/in6.h:30:8: note: originally defined here
In file included from /usr/include/arpa/inet.h:22:0,
                  from ./include/connman/inet.h:25,
                  from src/connman.h:128,
                  from src/tethering.c:40:
/usr/include/netinet/in.h:238:8: error: redefinition of 'struct 
sockaddr_in6'
In file included from /usr/include/linux/if_bridge.h:17:0,
                  from src/tethering.c:38:
/usr/include/linux/in6.h:46:8: note: originally defined here
In file included from /usr/include/arpa/inet.h:22:0,
                  from ./include/connman/inet.h:25,
                  from src/connman.h:128,
                  from src/tethering.c:40:
/usr/include/netinet/in.h:274:8: error: redefinition of 'struct ipv6_mreq'
In file included from /usr/include/linux/if_bridge.h:17:0,
                  from src/tethering.c:38:
/usr/include/linux/in6.h:54:8: note: originally defined here
make[1]: *** [src/src_connmand-tethering.o] Error 1


So I'm not sure it's the right one...


--
Thomas

^ permalink raw reply

* [PATCH V4 3/3] tuntap: allow polling/writing/reading when detached
From: Jason Wang @ 2013-01-15 10:28 UTC (permalink / raw)
  To: mst, davem, netdev, linux-kernel; +Cc: Jason Wang
In-Reply-To: <1358245726-16250-1-git-send-email-jasowang@redhat.com>

We forbid polling, writing and reading when the file were detached, this may
complex the user in several cases:

- when guest pass some buffers to vhost/qemu and then disable some queues,
  vhost/qemu needs to to its own cleanup which is complex. We can do this simply
  by allowing a user can still write to an disabled queue to handle this. And
  user can still do read but just nothing returned.
- align the polling behavior with macvtap which never fails when the queue is
  created. this can simplify the polling errors handling of its user (e.g vhost)

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/net/tun.c |   18 +++++++++++-------
 1 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index af372d0..eb68937 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -307,7 +307,7 @@ static void tun_flow_update(struct tun_struct *tun, u32 rxhash,
 
 	rcu_read_lock();
 
-	if (tun->numqueues == 1)
+	if (tun->numqueues == 1 || queue_index >= tun->numqueues)
 		goto unlock;
 
 	e = tun_flow_find(head, rxhash);
@@ -406,21 +406,21 @@ static void __tun_detach(struct tun_file *tfile, bool clean)
 
 	tun = rtnl_dereference(tfile->tun);
 
-	if (tun) {
+	if (tun && !tfile->detached) {
 		u16 index = tfile->queue_index;
 		BUG_ON(index >= tun->numqueues);
 		dev = tun->dev;
 
 		rcu_assign_pointer(tun->tfiles[index],
 				   tun->tfiles[tun->numqueues - 1]);
-		rcu_assign_pointer(tfile->tun, NULL);
 		ntfile = rtnl_dereference(tun->tfiles[index]);
 		ntfile->queue_index = index;
 
 		--tun->numqueues;
-		if (clean)
+		if (clean) {
+			rcu_assign_pointer(tfile->tun, NULL);
 			sock_put(&tfile->sk);
-		else
+		} else
 			tun_disable_queue(tun, tfile);
 
 		synchronize_net();
@@ -465,6 +465,10 @@ static void tun_detach_all(struct net_device *dev)
 		rcu_assign_pointer(tfile->tun, NULL);
 		--tun->numqueues;
 	}
+	list_for_each_entry(tfile, &tun->disabled, next) {
+		wake_up_all(&tfile->wq.wait);
+		rcu_assign_pointer(tfile->tun, NULL);
+	}
 	BUG_ON(tun->numqueues != 0);
 
 	synchronize_net();
@@ -491,7 +495,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file)
 	int err;
 
 	err = -EINVAL;
-	if (rtnl_dereference(tfile->tun))
+	if (rtnl_dereference(tfile->tun) && !tfile->detached)
 		goto out;
 
 	err = -EBUSY;
@@ -1795,7 +1799,7 @@ static int tun_set_queue(struct file *file, struct ifreq *ifr)
 			ret = tun_attach(tun, file);
 	} else if (ifr->ifr_flags & IFF_DETACH_QUEUE) {
 		tun = rtnl_dereference(tfile->tun);
-		if (!tun || !(tun->flags & TUN_TAP_MQ))
+		if (!tun || !(tun->flags & TUN_TAP_MQ) || tfile->detached)
 			ret = -EINVAL;
 		else
 			__tun_detach(tfile, false);
-- 
1.7.1

^ permalink raw reply related

* [PATCH V4 2/3] vhost_net: handle polling errors when setting backend
From: Jason Wang @ 2013-01-15 10:28 UTC (permalink / raw)
  To: mst, davem, netdev, linux-kernel; +Cc: Jason Wang
In-Reply-To: <1358245726-16250-1-git-send-email-jasowang@redhat.com>

Currently, the polling errors were ignored, which can lead following issues:

- vhost remove itself unconditionally from waitqueue when stopping the poll,
  this may crash the kernel since the previous attempt of starting may fail to
  add itself to the waitqueue
- userspace may think the backend were successfully set even when the polling
  failed.

Solve this by:

- check poll->wqh before trying to remove from waitqueue
- report polling errors in vhost_poll_start(), tx_poll_start(), the return value
  will be checked and returned when userspace want to set the backend

After this fix, there still could be a polling failure after backend is set, it
will addressed by the next patch.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/vhost/net.c   |   27 ++++++++++++++++++---------
 drivers/vhost/vhost.c |   18 +++++++++++++++---
 drivers/vhost/vhost.h |    2 +-
 3 files changed, 34 insertions(+), 13 deletions(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index d10ad6f..959b1cd 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -165,12 +165,16 @@ static void tx_poll_stop(struct vhost_net *net)
 }
 
 /* Caller must have TX VQ lock */
-static void tx_poll_start(struct vhost_net *net, struct socket *sock)
+static int tx_poll_start(struct vhost_net *net, struct socket *sock)
 {
+	int ret;
+
 	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;
+		return 0;
+	ret = vhost_poll_start(net->poll + VHOST_NET_VQ_TX, sock->file);
+	if (!ret)
+		net->tx_poll_state = VHOST_NET_POLL_STARTED;
+	return ret;
 }
 
 /* In case of DMA done not in order in lower device driver for some reason.
@@ -642,20 +646,23 @@ static void vhost_net_disable_vq(struct vhost_net *n,
 		vhost_poll_stop(n->poll + VHOST_NET_VQ_RX);
 }
 
-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;
+	int ret;
 
 	sock = rcu_dereference_protected(vq->private_data,
 					 lockdep_is_held(&vq->mutex));
 	if (!sock)
-		return;
+		return 0;
 	if (vq == n->vqs + VHOST_NET_VQ_TX) {
 		n->tx_poll_state = VHOST_NET_POLL_STOPPED;
-		tx_poll_start(n, sock);
+		ret = tx_poll_start(n, sock);
 	} else
-		vhost_poll_start(n->poll + VHOST_NET_VQ_RX, sock->file);
+		ret = vhost_poll_start(n->poll + VHOST_NET_VQ_RX, sock->file);
+
+	return ret;
 }
 
 static struct socket *vhost_net_stop_vq(struct vhost_net *n,
@@ -833,7 +840,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..9759249 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -77,26 +77,38 @@ 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);
 }
 
 /* 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;
 
 	mask = file->f_op->poll(file, &poll->table);
 	if (mask)
 		vhost_poll_wakeup(&poll->wait, 0, 0, (void *)mask);
+	if (mask & POLLERR) {
+		if (poll->wqh)
+			remove_wait_queue(poll->wqh, &poll->wait);
+		ret = -EINVAL;
+	}
+
+	return ret;
 }
 
 /* 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 (poll->wqh) {
+		remove_wait_queue(poll->wqh, &poll->wait);
+		poll->wqh = NULL;
+	}
 }
 
 static bool vhost_work_seq_done(struct vhost_dev *dev, struct vhost_work *work,
@@ -792,7 +804,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

^ permalink raw reply related

* [PATCH V4 1/3] vhost_net: correct error handling in vhost_net_set_backend()
From: Jason Wang @ 2013-01-15 10:28 UTC (permalink / raw)
  To: mst, davem, netdev, linux-kernel; +Cc: Jason Wang
In-Reply-To: <1358245726-16250-1-git-send-email-jasowang@redhat.com>

Currently, when vhost_init_used() fails the sock refcnt and ubufs were
leaked. Correct this by calling vhost_init_used() before assign ubufs and
restore the oldsock when it fails.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/vhost/net.c |   16 +++++++++++-----
 1 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index ebd08b2..d10ad6f 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -827,15 +827,16 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
 			r = PTR_ERR(ubufs);
 			goto err_ubufs;
 		}
-		oldubufs = vq->ubufs;
-		vq->ubufs = ubufs;
+
 		vhost_net_disable_vq(n, vq);
 		rcu_assign_pointer(vq->private_data, sock);
-		vhost_net_enable_vq(n, vq);
-
 		r = vhost_init_used(vq);
 		if (r)
-			goto err_vq;
+			goto err_used;
+		vhost_net_enable_vq(n, vq);
+
+		oldubufs = vq->ubufs;
+		vq->ubufs = ubufs;
 
 		n->tx_packets = 0;
 		n->tx_zcopy_err = 0;
@@ -859,6 +860,11 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
 	mutex_unlock(&n->dev.mutex);
 	return 0;
 
+err_used:
+	rcu_assign_pointer(vq->private_data, oldsock);
+	vhost_net_enable_vq(n, vq);
+	if (ubufs)
+		vhost_ubuf_put_and_wait(ubufs);
 err_ubufs:
 	fput(sock->file);
 err_vq:
-- 
1.7.1

^ permalink raw reply related

* [PATCH V4 0/3] handle polling erros in vhost/vhost_net
From: Jason Wang @ 2013-01-15 10:28 UTC (permalink / raw)
  To: mst, davem, netdev, linux-kernel; +Cc: Jason Wang

This is an update version of last version to fix the handling of polling errors
in vhost/vhost_net.

Currently, vhost and vhost_net ignore polling errors which can lead kernel
crashing when it tries to remove itself from waitqueue after the polling
failure. Fix this by:

- examing the POLLERR when set backend and report erros to userspace
- let tun always add to waitqueue in .poll() after the queue is created even if
  it was detached.

Changes from V3:
- make a smaller patch that doesn't touch the whole polling state and only check
  the polliner errors in backend setting.
- add a patch that allows tuntap to do polling/reading/writing when detached
  which could simplify the work of its user.

Changes from v2:
- check poll->wqh instead of the wrong assumption about POLLERR and waitqueue
- drop the whole tx polling state check since it was replaced by the wqh
  checking
- drop the buggy tuntap patch

Changes from v1:
- restore the state before the ioctl when vhost_init_used() fails
- log the error when meet polling errors in the data path
- don't put into waitqueue when tun_chr_poll() return POLLERR

Jason Wang (3):
  vhost_net: correct error handling in vhost_net_set_backend()
  vhost_net: handle polling errors when setting backend
  tuntap: allow polling/writing/reading when detached

 drivers/net/tun.c     |   18 +++++++++++-------
 drivers/vhost/net.c   |   41 ++++++++++++++++++++++++++++-------------
 drivers/vhost/vhost.c |   18 +++++++++++++++---
 drivers/vhost/vhost.h |    2 +-
 4 files changed, 55 insertions(+), 24 deletions(-)

^ permalink raw reply

* Re: [PATCH 1/1] net: ipv4: refine the logic for determining daddr in ip_queue_xmit.
From: Cong Ding @ 2013-01-15 10:12 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Harvey Yang, David S. Miller, Alexey Kuznetsov, James Morris,
	Hideaki YOSHIFUJI, netdev, linux-kernel
In-Reply-To: <1358225829.8744.4508.camel@edumazet-glaptop>

On Mon, Jan 14, 2013 at 08:57:09PM -0800, Eric Dumazet wrote:
> On Tue, 2013-01-15 at 12:47 +0800, Harvey Yang wrote:
> > The destination address daddr is faddr if source route option is set, otherwise it is inet_daddr. So use if-else to assign the value.
> > 
> > Signed-off-by: Harvey Yang <harvey.huawei.yang@gmail.com>
> > ---
> >  net/ipv4/ip_output.c |    4 ++--
> >  1 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
> > index 3e98ed2..22b738c 100644
> > --- a/net/ipv4/ip_output.c
> > +++ b/net/ipv4/ip_output.c
> > @@ -349,10 +349,10 @@ int ip_queue_xmit(struct sk_buff *skb, struct flowi *fl)
> >  		__be32 daddr;
> >  
> >  		/* Use correct destination address if we have options. */
> > -		daddr = inet->inet_daddr;
> >  		if (inet_opt && inet_opt->opt.srr)
> >  			daddr = inet_opt->opt.faddr;
> > -
> > +		else
> > +			daddr = inet->inet_daddr;
> >  		/* If this fails, retransmit mechanism of transport layer will
> >  		 * keep trying until route appears or the connection times
> >  		 * itself out.
> 
> Current code is faster, srr is not often used.
If you turn on O2 option in GCC, both versions behave the same (generate same
binary code). If srr is not often used, why not use unlikely() macro?
- cong

^ permalink raw reply

* Re: the patch "bridge: export multicast database via netlink" broke kernel 3.8 uapi (was: Re: [libvirt] if_bridge.h: include in6.h for struct in6_addr use)
From: Cong Wang @ 2013-01-15 10:11 UTC (permalink / raw)
  To: Thomas Backlund; +Cc: Eric Blake, netdev, linux-kernel, libvirt-list, tgraf
In-Reply-To: <50F52956.50201@mageia.org>

On Tue, 2013-01-15 at 12:03 +0200, Thomas Backlund wrote:
> Eric Blake skrev 15.1.2013 01:57:
> > On 01/13/2013 01:05 PM, Thomas Backlund wrote:
> >> Thomas Backlund skrev 13.1.2013 20:38:
> >>> patch both inline and attached as thunderbird tends to mess up ...
> >>
> >>> -----
> >>>
> >>> if_bridge.h uses struct in6_addr ip6; but does not include the in6.h
> >>> header.
> >>>
> >>> Found by trying to build libvirt and connman against 3.8-rc3 headers.
> >>>
> >>
> >> Ok,
> >> ignore this patch, it's not the correct fix as it introduces
> >> redefinitions...
> >>
> >> Btw, the error that I hit that made me suggest this fix was libvirt
> >> config check bailing out:
> >>
> >> config.log:/usr/include/linux/if_bridge.h:173:20: error: field 'ip6' has
> >> incomplete type
> >
> > Hmm, just now noticing this thread, after independently hitting and and
> > having to work around the same problem in libvirt:
> > https://www.redhat.com/archives/libvir-list/2013-January/msg00930.html
> > https://bugzilla.redhat.com/show_bug.cgi?id=895141
> 
> 
> Yep,
> 
> and the commit breaking uapi headers is by using "struct in6_addr ip6" is:
> 
> 
>  From ee07c6e7a6f8a25c18f0a6b18152fbd7499245f6 Mon Sep 17 00:00:00 2001
> From: Cong Wang <amwang@redhat.com>
> Date: Fri, 7 Dec 2012 00:04:48 +0000
> Subject: [PATCH] bridge: export multicast database via netlink

Does the following patch help?

$ git diff include/uapi/linux/if_bridge.h
diff --git a/include/uapi/linux/if_bridge.h
b/include/uapi/linux/if_bridge.h
index 5db2975..653db23 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -14,6 +14,7 @@
 #define _UAPI_LINUX_IF_BRIDGE_H
 
 #include <linux/types.h>
+#include <linux/in6.h>
 
 #define SYSFS_BRIDGE_ATTR      "bridge"
 #define SYSFS_BRIDGE_FDB       "brforward"

^ permalink raw reply related

* [libvirt] the patch "bridge: export multicast database via netlink" broke kernel 3.8 uapi (was: Re: if_bridge.h: include in6.h for struct in6_addr use)
From: Thomas Backlund @ 2013-01-15 10:03 UTC (permalink / raw)
  To: amwang; +Cc: tgraf, netdev, linux-kernel, libvirt-list
In-Reply-To: <50F49B7E.50906@redhat.com>

Eric Blake skrev 15.1.2013 01:57:
> On 01/13/2013 01:05 PM, Thomas Backlund wrote:
>> Thomas Backlund skrev 13.1.2013 20:38:
>>> patch both inline and attached as thunderbird tends to mess up ...
>>
>>> -----
>>>
>>> if_bridge.h uses struct in6_addr ip6; but does not include the in6.h
>>> header.
>>>
>>> Found by trying to build libvirt and connman against 3.8-rc3 headers.
>>>
>>
>> Ok,
>> ignore this patch, it's not the correct fix as it introduces
>> redefinitions...
>>
>> Btw, the error that I hit that made me suggest this fix was libvirt
>> config check bailing out:
>>
>> config.log:/usr/include/linux/if_bridge.h:173:20: error: field 'ip6' has
>> incomplete type
>
> Hmm, just now noticing this thread, after independently hitting and and
> having to work around the same problem in libvirt:
> https://www.redhat.com/archives/libvir-list/2013-January/msg00930.html
> https://bugzilla.redhat.com/show_bug.cgi?id=895141


Yep,

and the commit breaking uapi headers is by using "struct in6_addr ip6" is:


 From ee07c6e7a6f8a25c18f0a6b18152fbd7499245f6 Mon Sep 17 00:00:00 2001
From: Cong Wang <amwang@redhat.com>
Date: Fri, 7 Dec 2012 00:04:48 +0000
Subject: [PATCH] bridge: export multicast database via netlink

V5: fix two bugs pointed out by Thomas
     remove seq check for now, mark it as TODO

V4: remove some useless #include
     some coding style fix

V3: drop debugging printk's
     update selinux perm table as well

V2: drop patch 1/2, export ifindex directly
     Redesign netlink attributes
     Improve netlink seq check
     Handle IPv6 addr as well

This patch exports bridge multicast database via netlink
message type RTM_GETMDB. Similar to fdb, but currently bridge-specific.
We may need to support modify multicast database too (RTM_{ADD,DEL}MDB).

(Thanks to Thomas for patient reviews)

Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Stephen Hemminger <shemminger@vyatta.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Thomas Graf <tgraf@suug.ch>
Cc: Jesper Dangaard Brouer <brouer@redhat.com>
Signed-off-by: Cong Wang <amwang@redhat.com>
Acked-by: Thomas Graf <tgraf@suug.ch>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
  include/uapi/linux/if_bridge.h |   55 ++++++++++++++
  include/uapi/linux/rtnetlink.h |    3 +
  net/bridge/Makefile            |    2 +-
  net/bridge/br_mdb.c            |  163 
++++++++++++++++++++++++++++++++++++++++
  net/bridge/br_multicast.c      |    1 +
  net/bridge/br_private.h        |    1 +
  security/selinux/nlmsgtab.c    |    1 +
  7 files changed, 225 insertions(+), 1 deletions(-)
  create mode 100644 net/bridge/br_mdb.c

--
Thomas

^ permalink raw reply

* RE: [PATCH 0/5]: soreuseport: Bind multiple sockets to the same port
From: David Laight @ 2013-01-15  9:34 UTC (permalink / raw)
  To: Tom Herbert, netdev, davem; +Cc: netdev, eric.dumazet
In-Reply-To: <alpine.DEB.2.00.1301141138440.959@pokey.mtv.corp.google.com>



> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On Behalf Of Tom Herbert
> Sent: 14 January 2013 20:00
> To: netdev@vger.kernel.org; davem@davemloft.net
> Cc: netdev@markandruth.co.uk; eric.dumazet@gmail.com
> Subject: [PATCH 0/5]: soreuseport: Bind multiple sockets to the same port
> 
> Rebasing the soreuseport patches to 3.8.  No material changes since
> first posted.
> ---
> These patches implements so_reuseport (SO_REUSEPORT socket option) for
> TCP and UDP.  For TCP, so_reuseport allows multiple listener sockets
> to be bound to the same port.
>....
> 
> The motivating case for so_resuseport in TCP would be something like
> a web server binding to port 80 running with multiple threads, ...
> 2) accept on a single
> listener socket from multiple threads
> In case #2, the proportion of connections accepted per thread tends
> to be uneven under high connection load (assuming simple event loop:
> while (1) { accept(); process() }, wakeup does not promote fairness
> among the sockets.  We have seen the  disproportion to be as high
> as 3:1 ratio between thread accepting most connections and the one
> accepting the fewest.  With so_reusport the distribution is
> uniform.

Hmmm.... do you need that sort of fairness between the threads?

If one request takes longer than average to process, then you
don't want other requests to be delayed when there are other
idle worker processes.

Also having the same thread normally collect a request would
make it more likely that the required code/data be in the
cache of the cpu (assuming that the main reason for multiple
threads is to load balance over multiple cpus, and with the
threads tied to a single cpu).

If there are a lot of processes sleeping in accept() (on the same
socket) it might be worth looking at which is actually woken
when a new connection arrives. If they are sleeping in poll/select
it is probably more difficult (but not impossible) to avoid waking
all the processes for every incoming connection.

	David

^ permalink raw reply

* [Patch net-next v4] netpoll: fix a rtnl lock assertion failure
From: Cong Wang @ 2013-01-15  9:34 UTC (permalink / raw)
  To: netdev; +Cc: Eric Dumazet, Jiri Pirko, David S. Miller, Cong Wang

From: Cong Wang <amwang@redhat.com>

v4: hold rtnl lock for the whole netpoll_setup()
v3: remove the comment
v2: use RCU read lock

This patch fixes the following warning:

[   72.013864] RTNL: assertion failed at net/core/dev.c (4955)
[   72.017758] Pid: 668, comm: netpoll-prep-v6 Not tainted 3.8.0-rc1+ #474
[   72.019582] Call Trace:
[   72.020295]  [<ffffffff8176653d>] netdev_master_upper_dev_get+0x35/0x58
[   72.022545]  [<ffffffff81784edd>] netpoll_setup+0x61/0x340
[   72.024846]  [<ffffffff815d837e>] store_enabled+0x82/0xc3
[   72.027466]  [<ffffffff815d7e51>] netconsole_target_attr_store+0x35/0x37
[   72.029348]  [<ffffffff811c3479>] configfs_write_file+0xe2/0x10c
[   72.030959]  [<ffffffff8115d239>] vfs_write+0xaf/0xf6
[   72.032359]  [<ffffffff81978a05>] ? sysret_check+0x22/0x5d
[   72.033824]  [<ffffffff8115d453>] sys_write+0x5c/0x84
[   72.035328]  [<ffffffff819789d9>] system_call_fastpath+0x16/0x1b

In case of other races, hold rtnl lock for the entire netpoll_setup() function.

Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Jiri Pirko <jiri@resnulli.us>
Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 9f05067..a5ad1c1 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -1048,11 +1048,13 @@ int netpoll_setup(struct netpoll *np)
 	struct in_device *in_dev;
 	int err;
 
+	rtnl_lock();
 	if (np->dev_name)
-		ndev = dev_get_by_name(&init_net, np->dev_name);
+		ndev = __dev_get_by_name(&init_net, np->dev_name);
 	if (!ndev) {
 		np_err(np, "%s doesn't exist, aborting\n", np->dev_name);
-		return -ENODEV;
+		err = -ENODEV;
+		goto unlock;
 	}
 
 	if (netdev_master_upper_dev_get(ndev)) {
@@ -1066,15 +1068,14 @@ int netpoll_setup(struct netpoll *np)
 
 		np_info(np, "device %s not up yet, forcing it\n", np->dev_name);
 
-		rtnl_lock();
 		err = dev_open(ndev);
-		rtnl_unlock();
 
 		if (err) {
 			np_err(np, "failed to open %s\n", ndev->name);
 			goto put;
 		}
 
+		rtnl_unlock();
 		atleast = jiffies + HZ/10;
 		atmost = jiffies + carrier_timeout * HZ;
 		while (!netif_carrier_ok(ndev)) {
@@ -1094,16 +1095,14 @@ int netpoll_setup(struct netpoll *np)
 			np_notice(np, "carrier detect appears untrustworthy, waiting 4 seconds\n");
 			msleep(4000);
 		}
+		rtnl_lock();
 	}
 
 	if (!np->local_ip.ip) {
 		if (!np->ipv6) {
-			rcu_read_lock();
-			in_dev = __in_dev_get_rcu(ndev);
-
+			in_dev = __in_dev_get_rtnl(ndev);
 
 			if (!in_dev || !in_dev->ifa_list) {
-				rcu_read_unlock();
 				np_err(np, "no IP address for %s, aborting\n",
 				       np->dev_name);
 				err = -EDESTADDRREQ;
@@ -1111,14 +1110,12 @@ int netpoll_setup(struct netpoll *np)
 			}
 
 			np->local_ip.ip = in_dev->ifa_list->ifa_local;
-			rcu_read_unlock();
 			np_info(np, "local IP %pI4\n", &np->local_ip.ip);
 		} else {
 #if IS_ENABLED(CONFIG_IPV6)
 			struct inet6_dev *idev;
 
 			err = -EDESTADDRREQ;
-			rcu_read_lock();
 			idev = __in6_dev_get(ndev);
 			if (idev) {
 				struct inet6_ifaddr *ifp;
@@ -1133,7 +1130,6 @@ int netpoll_setup(struct netpoll *np)
 				}
 				read_unlock_bh(&idev->lock);
 			}
-			rcu_read_unlock();
 			if (err) {
 				np_err(np, "no IPv6 address for %s, aborting\n",
 				       np->dev_name);
@@ -1151,17 +1147,17 @@ int netpoll_setup(struct netpoll *np)
 	/* fill up the skb queue */
 	refill_skbs();
 
-	rtnl_lock();
 	err = __netpoll_setup(np, ndev, GFP_KERNEL);
-	rtnl_unlock();
-
 	if (err)
 		goto put;
 
+	rtnl_unlock();
 	return 0;
 
 put:
 	dev_put(ndev);
+unlock:
+	rtnl_unlock();
 	return err;
 }
 EXPORT_SYMBOL(netpoll_setup);

^ permalink raw reply related

* [PATCH net] net: qmi_wwan: add TP-LINK HSUPA Modem MA180
From: Bjørn Mork @ 2013-01-15  9:19 UTC (permalink / raw)
  To: netdev; +Cc: linux-usb, hockseng leow, Thomas Schäfer, Bjørn Mork
In-Reply-To: <201301122122.07307.tschaefer@t-online.de>

The driver description files gives these names to the vendor specific
functions on this modem:

 Diagnostics VID_2357&PID_0201&MI_00
 NMEA        VID_2357&PID_0201&MI_01
 Modem       VID_2357&PID_0201&MI_03
 Networkcard VID_2357&PID_0201&MI_04

The "Networkcard" function has been verified to support these QMI
services:
    ctl (1.3)
    wds (1.3)
    dms (1.2)
    nas (1.0)

Reported-by: Thomas Schäfer <tschaefer@t-online.de>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/usb/qmi_wwan.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 6a1ca50..c434108 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -459,6 +459,7 @@ static const struct usb_device_id products[] = {
 	{QMI_FIXED_INTF(0x1199, 0x68a2, 19)},	/* Sierra Wireless MC7710 in QMI mode */
 	{QMI_FIXED_INTF(0x1199, 0x901c, 8)},    /* Sierra Wireless EM7700 */
 	{QMI_FIXED_INTF(0x1bbb, 0x011e, 4)},	/* Telekom Speedstick LTE II (Alcatel One Touch L100V LTE) */
+	{QMI_FIXED_INTF(0x2357, 0x0201, 4)},	/* TP-LINK HSUPA Modem MA180 */
 
 	/* 4. Gobi 1000 devices */
 	{QMI_GOBI1K_DEVICE(0x05c6, 0x9212)},	/* Acer Gobi Modem Device */
-- 
1.7.10.4

^ permalink raw reply related


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