Netdev List
 help / color / mirror / Atom feed
* Re: can TCP socket send buffer be over used?
From: Mikael Abrahamsson @ 2010-08-04  8:21 UTC (permalink / raw)
  To: Jack Zhang; +Cc: netdev
In-Reply-To: <AANLkTikJvTvzuPhJ59RNcCGXPWP4EidaHx0=oUK8jjiy@mail.gmail.com>

On Wed, 4 Aug 2010, Jack Zhang wrote:

> Hi Mikael,
>
> Thanks for your reply. I'll definitely try that.
>
> Quick question though... the link I use in my test does not have any
> packet loss (it's a straight through cable between two PCs)... in this
> case, would TCP congestion window size affect the result at all?

I always mix up the different TCP windows, but I mean the windows size the 
sender will use max for the session. I'm not sure this is the same as the 
buffer you're tuning with your userspace option.

An easy test would be for you to set the userspace option to 64k, test 
what speeds you get, then you turn off window scaling in /proc, and try 
again. If you get wildly different results here (turning off window 
scaling limits the window to 64k) then the buffer you're tuning doesn't do 
what you think it does.

-- 
Mikael Abrahamsson    email: swmike@swm.pp.se

^ permalink raw reply

* [PATCH] cls_rsvp: add sanity check for the packet length
From: Changli Gao @ 2010-08-04  8:55 UTC (permalink / raw)
  To: Jamal Hadi Salim; +Cc: David S. Miller, netdev, Changli Gao

The packet length should be checked before the packet data is dereferenced.

Signed-off-by: Changli Gao <xiaosuo@gmail.com>
---
 net/sched/cls_rsvp.h |   10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/net/sched/cls_rsvp.h b/net/sched/cls_rsvp.h
index dd9414e..4fa119d 100644
--- a/net/sched/cls_rsvp.h
+++ b/net/sched/cls_rsvp.h
@@ -143,9 +143,17 @@ static int rsvp_classify(struct sk_buff *skb, struct tcf_proto *tp,
 	u8 tunnelid = 0;
 	u8 *xprt;
 #if RSVP_DST_LEN == 4
-	struct ipv6hdr *nhptr = ipv6_hdr(skb);
+	struct ipv6hdr *nhptr;
+
+	if (!pskb_may_pull(skb, skb_network_offset(skb) + sizeof(*nhptr)))
+		return -1;
+	nhptr = ipv6_hdr(skb);
 #else
 	struct iphdr *nhptr = ip_hdr(skb);
+
+	if (!pskb_may_pull(skb, skb_network_offset(skb) + sizeof(*nhptr)))
+		return -1;
+	nhptr = ip_hdr(skb);
 #endif
 
 restart:

^ permalink raw reply related

* Re: [RFC PATCH v8 00/16] Provide a zero-copy method on KVM virtio-net.
From: Arnd Bergmann @ 2010-08-04  8:56 UTC (permalink / raw)
  To: Dong, Eddie
  Cc: Shirley Ma, Xin, Xiaohui, netdev@vger.kernel.org,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org, mst@redhat.com,
	mingo@elte.hu, davem@davemloft.net, herbert@gondor.apana.org.au,
	jdike@linux.intel.com
In-Reply-To: <1A42CE6F5F474C41B63392A5F80372B228FCE7B1@shsmsx501.ccr.corp.intel.com>

On Wednesday 04 August 2010, Dong, Eddie wrote:
> Arnd Bergmann wrote:
> > On Friday 30 July 2010 17:51:52 Shirley Ma wrote:
> >> I think it should be less duplicated code in the kernel if we use
> >> macvtap to support what media passthrough driver here. Since macvtap
> >> has support virtio_net head and offloading already, the only missing
> >> func is zero copy. Also QEMU supports macvtap, we just need add a
> >> zero copy flag in option.
> > 
> > Yes, I fully agree and that was one of the intended directions for
> > macvtap to start with. Thank you so much for following up on that,
> > I've long been planning to work on macvtap zero-copy myself but it's
> > now lower on my priorities, so it's good to hear that you made
> > progress on it, even if there are still performance issues.
> > 
> 
> But zero-copy is a Linux generic feature that can be used by other
> VMMs as well if the BE service drivers want to incorporate.  If we
> can make mp device VMM-agnostic (it may be not yet in current patch),
> that will help Linux more.

But the tun/tap protocol is what most hypervisors use today on Linux,
and one of the design goals of macvtap was to keep that interface
so that everyone gets the features like zero-copy if that is added
to macvtap. The mp device interface is currently not supported by
anything else than vhost with these patches, and making it more
generic would turn the interface into a copy of macvtap.

	Arnd

^ permalink raw reply

* [PATCH] sch_sfq: add sanity check for the packet length
From: Changli Gao @ 2010-08-04  8:55 UTC (permalink / raw)
  To: Jamal Hadi Salim; +Cc: David S. Miller, netdev, Changli Gao

The packet length should be checked before the packet data is dereferenced.

Signed-off-by: Changli Gao <xiaosuo@gmail.com>
---
 net/sched/sch_sfq.c |   31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index c657628..018c528 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -122,7 +122,11 @@ static unsigned sfq_hash(struct sfq_sched_data *q, struct sk_buff *skb)
 	switch (skb->protocol) {
 	case htons(ETH_P_IP):
 	{
-		const struct iphdr *iph = ip_hdr(skb);
+		const struct iphdr *iph;
+
+		if (!pskb_may_pull(skb, skb_network_offset(skb) + sizeof(*iph)))
+			goto err;
+		iph = ip_hdr(skb);
 		h = (__force u32)iph->daddr;
 		h2 = (__force u32)iph->saddr ^ iph->protocol;
 		if (!(iph->frag_off&htons(IP_MF|IP_OFFSET)) &&
@@ -131,25 +135,34 @@ static unsigned sfq_hash(struct sfq_sched_data *q, struct sk_buff *skb)
 		     iph->protocol == IPPROTO_UDPLITE ||
 		     iph->protocol == IPPROTO_SCTP ||
 		     iph->protocol == IPPROTO_DCCP ||
-		     iph->protocol == IPPROTO_ESP))
+		     iph->protocol == IPPROTO_ESP) &&
+		     pskb_may_pull(skb, skb_network_offset(skb) +
+					iph->ihl * 4 + 4))
 			h2 ^= *(((u32*)iph) + iph->ihl);
 		break;
 	}
 	case htons(ETH_P_IPV6):
 	{
-		struct ipv6hdr *iph = ipv6_hdr(skb);
+		struct ipv6hdr *iph;
+
+		if (!pskb_may_pull(skb, skb_network_offset(skb) + sizeof(*iph)))
+			goto err;
+		iph = ipv6_hdr(skb);
 		h = (__force u32)iph->daddr.s6_addr32[3];
 		h2 = (__force u32)iph->saddr.s6_addr32[3] ^ iph->nexthdr;
-		if (iph->nexthdr == IPPROTO_TCP ||
-		    iph->nexthdr == IPPROTO_UDP ||
-		    iph->nexthdr == IPPROTO_UDPLITE ||
-		    iph->nexthdr == IPPROTO_SCTP ||
-		    iph->nexthdr == IPPROTO_DCCP ||
-		    iph->nexthdr == IPPROTO_ESP)
+		if ((iph->nexthdr == IPPROTO_TCP ||
+		     iph->nexthdr == IPPROTO_UDP ||
+		     iph->nexthdr == IPPROTO_UDPLITE ||
+		     iph->nexthdr == IPPROTO_SCTP ||
+		     iph->nexthdr == IPPROTO_DCCP ||
+		     iph->nexthdr == IPPROTO_ESP) &&
+		    pskb_may_pull(skb, skb_network_offset(skb) + sizeof(*iph) +
+				       4))
 			h2 ^= *(u32*)&iph[1];
 		break;
 	}
 	default:
+err:
 		h = (unsigned long)skb_dst(skb) ^ (__force u32)skb->protocol;
 		h2 = (unsigned long)skb->sk;
 	}

^ permalink raw reply related

* Re: can TCP socket send buffer be over used?
From: Bill Fink @ 2010-08-04  9:07 UTC (permalink / raw)
  To: Jack Zhang; +Cc: netdev
In-Reply-To: <AANLkTimhswtowB-TZ5Hj4KQXcf3gzjpxEJ8qO+PuWaPV@mail.gmail.com>

On Wed, 4 Aug 2010, Jack Zhang wrote:

> Hi Bill,
> 
> Thanks a lot for your help.
> 
> It does make sense!
> 
> As I'm writing this part into my master thesis, do you happen to know
> which part in the source code I can maybe use as a proof in the thesis
> that Linux uses 1/4 of the doubled buffer size for metadata?

Don't know about the source code, but from
Documentation/networking/ip-sysctl.txt:

tcp_adv_win_scale - INTEGER
	Count buffering overhead as bytes/2^tcp_adv_win_scale
	(if tcp_adv_win_scale > 0) or bytes-bytes/2^(-tcp_adv_win_scale),
	if it is <= 0.
	Default: 2

wizin% cat /proc/sys/net/ipv4/tcp_adv_win_scale 
2

For the oddity involving the 128 KB window case, it seems to have
something to do with the TCP receiver autotuning.  On a real
cross-country link (~80 ms RTT), the best to be expected is:

wizin% bc
scale=10
128*1024*8/0.080/10^6*3/2
19.6608000000

And an actual 60-second nuttcp test (which by default sets both
the sender and receiver socket buffer sizes):

netem1% nuttcp -T60 -i5 -w128 192.168.1.18
    8.8125 MB /   5.00 sec =   14.7849 Mbps     0 retrans
    9.2500 MB /   5.00 sec =   15.5189 Mbps     0 retrans
    9.1875 MB /   5.00 sec =   15.4141 Mbps     0 retrans
    9.5000 MB /   5.00 sec =   15.9384 Mbps     0 retrans
    9.1250 MB /   5.00 sec =   15.3092 Mbps     0 retrans
    9.1875 MB /   5.00 sec =   15.4141 Mbps     0 retrans
    9.4375 MB /   5.00 sec =   15.8335 Mbps     0 retrans
    9.3125 MB /   5.00 sec =   15.6238 Mbps     0 retrans
    9.3125 MB /   5.00 sec =   15.6238 Mbps     0 retrans
    9.1250 MB /   5.00 sec =   15.3092 Mbps     0 retrans
    9.1875 MB /   5.00 sec =   15.4141 Mbps     0 retrans
    9.4375 MB /   5.00 sec =   15.8335 Mbps     0 retrans

  111.0100 MB /  60.13 sec =   15.4867 Mbps 0 %TX 0 %RX 0 retrans 80.59 msRTT

But if I allow the receiver to do autotuning by specifying
a server window size of 0:

netem1% nuttcp -T60 -i5 -w128 -ws0 192.168.1.18
   14.3125 MB /   5.00 sec =   24.0123 Mbps     0 retrans
   15.5000 MB /   5.00 sec =   26.0047 Mbps     0 retrans
   15.5000 MB /   5.00 sec =   26.0047 Mbps     0 retrans
   15.5000 MB /   5.00 sec =   26.0047 Mbps     0 retrans
   15.3750 MB /   5.00 sec =   25.7950 Mbps     0 retrans
   15.3750 MB /   5.00 sec =   25.7950 Mbps     0 retrans
   15.5000 MB /   5.00 sec =   26.0047 Mbps     0 retrans
   15.5000 MB /   5.00 sec =   26.0047 Mbps     0 retrans
   15.5000 MB /   5.00 sec =   26.0047 Mbps     0 retrans
   15.3750 MB /   5.00 sec =   25.7950 Mbps     0 retrans
   15.5000 MB /   5.00 sec =   26.0047 Mbps     0 retrans
   15.3750 MB /   5.00 sec =   25.7950 Mbps     0 retrans

  184.3643 MB /  60.04 sec =   25.7609 Mbps 0 %TX 0 %RX 0 retrans 80.58 msRTT

This kind of makes sense since with autotuning, the receiver
is allowed to increase the socket buffer size beyond 128 KB.
One would have to tcpdump the packet flow to see what the
receiver's advertised TCP window was.  Rate throttling by
specifying the socket buffer size only seems to be truly
effective when done by the receiver, not when it's only
done on the sender side.

					-Bill

P.S.  BTW I've also seen cases (on some older kernels), where
      the window scale used was 1 more than it should have been,
      resulting in the receiver's advertised TCP window being
      twice what one would have expected.  tcpdump can also be
      used to verify proper functioning of the window scaling.



> Thanks,
> Jack
> 
> On 4 August 2010 01:20, Bill Fink <billfink@mindspring.com> wrote:
> > On Tue, 3 Aug 2010, Jack Zhang wrote:
> >
> >> Hi there,
> >>
> >> I'm doing experiments with (modified*) software iSCSI over a link with
> >> an emulated Round-Trip Time (RTT) of 100 ms by netem.
> >>
> >> For example, when I set the send buffer size to 128 KB, i could get a
> >> throughput up to 43 Mbps, which seems to be impossible as the (buffer
> >> size) / RTT is only 10 Mbps.
> >
> > I'm not sure what's going on with this first case.
> >
> >> And When I set the send buffer size to 512 KB, i can get a throughput
> >> up to 60 Mbps, which also seems to be impossible as the (buffer size)
> >> / RTT is only 40 Mbps.
> >
> > But this case seems just about right.  Linux doubles the requested
> > buffer size, then uses one quarter of that for overhead (not half),
> > so you effectively get 50% more than requested (2X * 3/4 = 1.5X).
> > Plugging your case into bc:
> >
> > wizin% bc
> > scale=10
> > 512*1024*8/0.100/10^6*3/2
> > 62.9145600000
> >
> >                                                -Bill
> >
> >
> >
> >> I understand that when the buffer size is set to 128 KB, I actually
> >> got a buffer of 256 KB as the kernel doubles the buffer size. I also
> >> understand that half the doubled buffer size is used for meta data
> >> instead of the actual data to be transferred. So basically the
> >> effective buffer sizes for the two examples  are just 128 KB and 512
> >> KB respectively.
> >>
> >> So I was confused because, theoretically, send buffers of 128 KB and
> >> 512 KB should achieve no more than 10 Mbps and 40 Mbps respectively
> >> but I was able to get way much more than the theoretical limit. So
> >> I was wondering is there any chance the send buffer can be "overused"?
> >> or there is some other mechanism inside TCP is doing some
> >> optimization?
> >>
> >> * the modification is to disable "TCP_NODELAY" , enable
> >> "use_clustering" for SCSI, and set different send buffer sizes for the
> >> TCP socket buffer.
> >>
> >> Any idea will be highly appreciated.
> >>
> >> Thanks a lot!

^ permalink raw reply

* Contect Fedex
From: Amber Juliana @ 2010-08-04  9:14 UTC (permalink / raw)
  To: info

Greetings
 
I have been waiting for you to contact me for your Confirmable Bank-draft of $750.000.00 United States Dollars,but I did not hear from 
you. Then I went and deposited the Draft with FedEx COURIER SERVICE,The only money you will send to the FedEX COURIER SERVICE to 
deliver your Draft direct to your postal Address in your country is ($150.00 US) Dollars only being Security Keeping Fee of the Courier 
Company.


1) FULL NAMES:
2) CONTACT ADDRESS:
3) PHONE NUMBER/FAX:
4) COUNTRY:
 
Contact Person:Mr Fred Osamudia
Phone number:+234 8139583375
Email Address:fedex001@gala.net



---- Nuova grafica e nuove funzionalità! Crea subito Gratis la tua nuova Casella di Posta  Katamail

^ permalink raw reply

* Re: [RFC PATCH] platform: Faciliatate the creation of pseduo-platform busses
From: Alan Cox @ 2010-08-04  9:37 UTC (permalink / raw)
  To: Patrick Pannuto
  Cc: linux-kernel, linux-arm-msm, linux-omap.vger.kernel.org,
	Greg Kroah-Hartman, damm, lethal, rjw, dtor, eric.y.miao, netdev
In-Reply-To: <4C58A7AA.8020007@codeaurora.org>

> 	(possibly in a header)
> 	#ifdef CONFIG_MY_BUS
> 	#define MY_BUS_TYPE	&my_bus_type
> 	#else
> 	#define MY_BUS_TYPE	NULL
> 	#endif
> 
> /drivers/my_driver.c
> 	static struct platform_driver my_driver = {
> 		.driver	= {
> 			.name	= "my-driver",
> 			.owner	= THIS_MODULE,
> 			.bus	= MY_BUS_TYPE,
> 		},
> 	};
> 
> Which will allow the same driver to easily to used on either
> the platform bus or the newly defined bus type.

At compile time. I suspect there is an argument for having an
"ANY_BUS_TYPE" value for the devices so that you can runtime wildcard
stuff which works whatever sub-bus it is hung off.

> I believe this to be a fairly elegant and simple solution to the
> problem, but humbly RFC

It's exactly what I need to tidy up the SCU IPC devices on the new Intel
MID platforms certainly. It's also a very small patch to achieve it

Alan

^ permalink raw reply

* PATCH 1/3] PPTP: PPP over IPv4 (Point-to-Point Tunneling Protocol)
From: xeb @ 2010-08-04 10:40 UTC (permalink / raw)
  To: netdev

This is patch 1/3 which contains gre demultiplexer driver source 
for demultiplexing gre packets with different gre version.

---
 include/net/gre.h |   18 ++++++
 net/ipv4/Kconfig  |    7 +++
 net/ipv4/Makefile |    1 +
 net/ipv4/gre.c    |  151 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 177 insertions(+), 0 deletions(-)

diff --git a/include/net/gre.h b/include/net/gre.h
new file mode 100644
index 0000000..31a0f76
--- /dev/null
+++ b/include/net/gre.h
@@ -0,0 +1,18 @@
+#ifndef __LINUX_GRE_H
+#define __LINUX_GRE_H
+
+#include <linux/skbuff.h>
+
+#define GREPROTO_CISCO		0
+#define GREPROTO_PPTP		1
+#define GREPROTO_MAX		2
+
+struct gre_protocol {
+	int		(*handler)(struct sk_buff *skb);
+	void	(*err_handler)(struct sk_buff *skb, u32 info);
+};
+
+int gre_add_protocol(const struct gre_protocol *proto, u8 version);
+int gre_del_protocol(const struct gre_protocol *proto, u8 version);
+
+#endif
diff --git a/net/ipv4/Kconfig b/net/ipv4/Kconfig
index 7c3a7d1..7458bda 100644
--- a/net/ipv4/Kconfig
+++ b/net/ipv4/Kconfig
@@ -215,8 +215,15 @@ config NET_IPIP
 	  be inserted in and removed from the running kernel whenever you
 	  want). Most people won't need this and can say N.
 
+config NET_IPGRE_DEMUX
+	tristate "IP: GRE demultiplexer"
+	help
+	 This is helper module to demultiplex GRE packets on GRE version field criteria.
+	 Required by ip_gre and pptp modules.
+
 config NET_IPGRE
 	tristate "IP: GRE tunnels over IP"
+	depends on NET_IPGRE_DEMUX
 	help
 	  Tunneling means encapsulating data of one protocol type within
 	  another protocol and sending it over a channel that understands the
diff --git a/net/ipv4/Makefile b/net/ipv4/Makefile
index 80ff87c..4978d22 100644
--- a/net/ipv4/Makefile
+++ b/net/ipv4/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_PROC_FS) += proc.o
 obj-$(CONFIG_IP_MULTIPLE_TABLES) += fib_rules.o
 obj-$(CONFIG_IP_MROUTE) += ipmr.o
 obj-$(CONFIG_NET_IPIP) += ipip.o
+obj-$(CONFIG_NET_IPGRE_DEMUX) += gre.o
 obj-$(CONFIG_NET_IPGRE) += ip_gre.o
 obj-$(CONFIG_SYN_COOKIES) += syncookies.o
 obj-$(CONFIG_INET_AH) += ah4.o
diff --git a/net/ipv4/gre.c b/net/ipv4/gre.c
new file mode 100644
index 0000000..993f3cf
--- /dev/null
+++ b/net/ipv4/gre.c
@@ -0,0 +1,151 @@
+/*
+ *	GRE over IPv4 demultiplexer driver
+ *
+ *	Authors: Dmitry Kozlov (xeb@mail.ru)
+ *
+ *	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.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/kmod.h>
+#include <linux/skbuff.h>
+#include <linux/in.h>
+#include <linux/netdevice.h>
+#include <linux/version.h>
+#include <linux/spinlock.h>
+#include <net/protocol.h>
+#include <net/gre.h>
+
+
+const struct gre_protocol *gre_proto[GREPROTO_MAX] ____cacheline_aligned_in_smp;
+static DEFINE_RWLOCK(gre_proto_lock);
+
+int gre_add_protocol(const struct gre_protocol *proto, u8 version)
+{
+	int ret;
+
+	if (version >= GREPROTO_MAX)
+		return -1;
+	
+	write_lock_bh(&gre_proto_lock);
+	if (gre_proto[version]) {
+		ret = -1;
+	} else {
+		gre_proto[version]=proto;
+		ret = 0;
+	}
+	write_unlock_bh(&gre_proto_lock);
+
+	return ret;
+}
+int gre_del_protocol(const struct gre_protocol *proto, u8 version)
+{
+	int ret;
+
+	if (version >= GREPROTO_MAX)
+		return -1;
+
+	write_lock_bh(&gre_proto_lock);
+	if (gre_proto[version] == proto) {
+		gre_proto[version] = NULL;
+		ret = 0;
+	} else {
+		ret = -1;
+	}
+	write_unlock_bh(&gre_proto_lock);
+
+	return ret;
+}
+static int gre_rcv(struct sk_buff *skb)
+{
+	u8 ver;
+	int ret;
+
+	if (!pskb_may_pull(skb, 12))
+		goto drop_nolock;
+
+	ver = skb->data[1]&0x7f;
+	if (ver >= GREPROTO_MAX)
+		goto drop_nolock;
+	
+	read_lock(&gre_proto_lock);
+	if (!gre_proto[ver] || !gre_proto[ver]->handler)
+		goto drop;
+
+	ret = gre_proto[ver]->handler(skb);
+	read_unlock(&gre_proto_lock);
+
+	return ret;
+
+drop:
+	read_unlock(&gre_proto_lock);
+drop_nolock:
+	kfree_skb(skb);
+	return NET_RX_DROP;
+}
+static void gre_err(struct sk_buff *skb, u32 info)
+{
+	u8 ver;
+
+	printk("err\n");
+
+	if (!pskb_may_pull(skb, 12))
+		goto drop_nolock;
+
+	ver=skb->data[1];
+	if (ver>=GREPROTO_MAX)
+		goto drop_nolock;
+		
+	read_lock(&gre_proto_lock);
+	if (!gre_proto[ver] || !gre_proto[ver]->err_handler)
+		goto drop;
+
+	gre_proto[ver]->err_handler(skb,info);
+	read_unlock(&gre_proto_lock);
+
+	return;
+
+drop:
+	read_unlock(&gre_proto_lock);
+drop_nolock:
+	kfree_skb(skb);
+}
+
+
+static struct net_protocol net_gre_protocol = {
+	.handler	= gre_rcv,
+	.err_handler	=	gre_err,
+//	.netns_ok=1,
+};
+
+static int __init gre_init(void)
+{
+	printk(KERN_INFO "GRE over IPv4 demultiplexor driver");
+	
+	if (inet_add_protocol(&net_gre_protocol, IPPROTO_GRE) < 0) {
+		printk(KERN_INFO "gre: can't add protocol\n");
+		return -EAGAIN;
+	}
+
+	return 0;
+}
+
+static void __exit gre_exit(void)
+{
+	inet_del_protocol(&net_gre_protocol, IPPROTO_GRE);
+}
+
+module_init(gre_init);
+module_exit(gre_exit);
+
+MODULE_DESCRIPTION("GRE over IPv4 demultiplexer driver");
+MODULE_AUTHOR("D. Kozlov (xeb@mail.ru)");
+MODULE_LICENSE("GPL");
+EXPORT_SYMBOL_GPL(gre_add_protocol);
+EXPORT_SYMBOL_GPL(gre_del_protocol);


^ permalink raw reply related

* [PATCH 2/3] PPTP: PPP over IPv4 (Point-to-Point Tunneling Protocol)
From: xeb @ 2010-08-04 10:45 UTC (permalink / raw)
  To: netdev

This is patch 2/3 which contains pptp driver source.

---
 drivers/net/Kconfig      |   11 +
 drivers/net/Makefile     |    1 +
 drivers/net/pptp.c       |  821 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/if_pppox.h |   20 +-
 4 files changed, 852 insertions(+), 1 deletions(-)

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index ce2fcdd..2fa0516 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -3167,6 +3167,17 @@ config PPPOE
 	  which contains instruction on how to use this driver (under 
 	  the heading "Kernel mode PPPoE").
 
+config PPTP
+	tristate "PPP over IPv4 (PPTP) (EXPERIMENTAL)"
+	depends on EXPERIMENTAL && PPP && NET_IPGRE_DEMUX
+	help
+	  Support for PPP over IPv4.(Point-to-Point Tunneling Protocol)
+
+	  This driver requires pppd plugin to work in client mode or
+	  modified pptpd (poptop) to work in server mode.
+	  See http://accel-pptp.sourceforge.net/ for information how to
+	  utilize this module.
+
 config PPPOATM
 	tristate "PPP over ATM"
 	depends on ATM && PPP
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index 0a0512a..b33fef1 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -162,6 +162,7 @@ obj-$(CONFIG_PPP_BSDCOMP) += bsd_comp.o
 obj-$(CONFIG_PPP_MPPE) += ppp_mppe.o
 obj-$(CONFIG_PPPOE) += pppox.o pppoe.o
 obj-$(CONFIG_PPPOL2TP) += pppox.o
+obj-$(CONFIG_PPTP) += pppox.o pptp.o
 
 obj-$(CONFIG_SLIP) += slip.o
 obj-$(CONFIG_SLHC) += slhc.o
diff --git a/drivers/net/pptp.c b/drivers/net/pptp.c
new file mode 100644
index 0000000..0c2dbe9
--- /dev/null
+++ b/drivers/net/pptp.c
@@ -0,0 +1,821 @@
+/*
+ *  Point-to-Point Tunneling Protocol for Linux
+ *
+ *	Authors: Dmitry Kozlov <xeb@mail.ru>
+ *
+ *	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.
+ *
+ */
+
+#include <linux/string.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/errno.h>
+#include <linux/netdevice.h>
+#include <linux/net.h>
+#include <linux/skbuff.h>
+#include <linux/init.h>
+#include <linux/ppp_channel.h>
+#include <linux/ppp_defs.h>
+#include <linux/if_pppox.h>
+#include <linux/if_ppp.h>
+#include <linux/notifier.h>
+#include <linux/file.h>
+#include <linux/in.h>
+#include <linux/ip.h>
+#include <linux/netfilter.h>
+#include <linux/netfilter_ipv4.h>
+#include <linux/version.h>
+#include <linux/spinlock.h>
+
+#include <net/sock.h>
+#include <net/protocol.h>
+#include <net/ip.h>
+#include <net/icmp.h>
+#include <net/route.h>
+#include <net/gre.h>
+
+#include <asm/uaccess.h>
+
+#define DEBUG
+
+#define PPTP_DRIVER_VERSION "0.8.4"
+
+static int log_level = 0;
+static int log_packets = 10;
+
+#define MAX_CALLID 65535
+#define PPP_LCP_ECHOREQ 0x09
+#define PPP_LCP_ECHOREP 0x0A
+#define SC_RCV_BITS	(SC_RCV_B7_1|SC_RCV_B7_0|SC_RCV_ODDP|SC_RCV_EVNP)
+
+static unsigned long *callid_bitmap = NULL;
+static struct pppox_sock **callid_sock=NULL;
+
+
+static DEFINE_RWLOCK(chan_lock);
+
+static int pptp_xmit(struct ppp_channel *chan, struct sk_buff *skb);
+static int pptp_ppp_ioctl(struct ppp_channel *chan, unsigned int cmd,
+			   unsigned long arg);
+static int pptp_rcv_core(struct sock *sk,struct sk_buff *skb);
+
+static struct ppp_channel_ops pptp_chan_ops = {
+	.start_xmit = pptp_xmit,
+	.ioctl      = pptp_ppp_ioctl,
+};
+
+
+#define MISSING_WINDOW 20
+#define WRAPPED( curseq, lastseq) \
+    ((((curseq) & 0xffffff00) == 0) && \
+     (((lastseq) & 0xffffff00 ) == 0xffffff00))
+
+/* gre header structure: -------------------------------------------- */
+
+#define PPTP_GRE_PROTO  0x880B
+#define PPTP_GRE_VER    0x1
+
+#define PPTP_GRE_FLAG_C	0x80
+#define PPTP_GRE_FLAG_R	0x40
+#define PPTP_GRE_FLAG_K	0x20
+#define PPTP_GRE_FLAG_S	0x10
+#define PPTP_GRE_FLAG_A	0x80
+
+#define PPTP_GRE_IS_C(f) ((f)&PPTP_GRE_FLAG_C)
+#define PPTP_GRE_IS_R(f) ((f)&PPTP_GRE_FLAG_R)
+#define PPTP_GRE_IS_K(f) ((f)&PPTP_GRE_FLAG_K)
+#define PPTP_GRE_IS_S(f) ((f)&PPTP_GRE_FLAG_S)
+#define PPTP_GRE_IS_A(f) ((f)&PPTP_GRE_FLAG_A)
+
+#define PPTP_HEADER_OVERHEAD (2+sizeof(struct pptp_gre_header))
+struct pptp_gre_header {
+  u_int8_t flags;		/* bitfield */
+  u_int8_t ver;			/* should be PPTP_GRE_VER (enhanced GRE) */
+  u_int16_t protocol;		/* should be PPTP_GRE_PROTO (ppp-encaps) */
+  u_int16_t payload_len;	/* size of ppp payload, not inc. gre header */
+  u_int16_t call_id;		/* peer's call_id for this session */
+  u_int32_t seq;		/* sequence number.  Present if S==1 */
+  u_int32_t ack;		/* seq number of highest packet recieved by */
+  				/*  sender in this session */
+};
+
+static struct pppox_sock * lookup_chan(__u16 call_id, __be32 s_addr)
+{
+	struct pppox_sock *sock;
+	struct pptp_opt *opt;
+	
+	read_lock(&chan_lock);
+	sock = callid_sock[call_id];
+	if (sock) {
+		opt = &sock->proto.pptp;
+		if (opt->dst_addr.sin_addr.s_addr != s_addr) sock = NULL;
+		else sock_hold(sk_pppox(sock));
+	}
+	read_unlock(&chan_lock);
+	
+	return sock;
+}
+
+static int lookup_chan_dst(__u16 call_id, __be32 d_addr)
+{
+	struct pppox_sock *sock;
+	struct pptp_opt *opt;
+	int i;
+	
+	read_lock(&chan_lock);
+	for(i = find_next_bit(callid_bitmap,MAX_CALLID,1); i < MAX_CALLID; i = find_next_bit(callid_bitmap,MAX_CALLID,i+1)) {
+	    sock = callid_sock[i];
+	    opt = &sock->proto.pptp;
+	    if (opt->dst_addr.call_id == call_id && opt->dst_addr.sin_addr.s_addr == d_addr) break;
+	}
+	read_unlock(&chan_lock);
+	
+	return i < MAX_CALLID;
+}
+
+static int add_chan(struct pppox_sock *sock)
+{
+	static int call_id = 0;
+	int res = -1;
+
+	write_lock_bh(&chan_lock);
+	
+	if (!sock->proto.pptp.src_addr.call_id)	{
+	    call_id = find_next_zero_bit(callid_bitmap,MAX_CALLID,call_id+1);
+	    if (call_id == MAX_CALLID)
+				call_id = find_next_zero_bit(callid_bitmap,MAX_CALLID,1);
+	    sock->proto.pptp.src_addr.call_id = call_id;
+	}
+	else if (test_bit(sock->proto.pptp.src_addr.call_id,callid_bitmap))
+		goto exit;
+	
+	set_bit(sock->proto.pptp.src_addr.call_id,callid_bitmap);
+	callid_sock[sock->proto.pptp.src_addr.call_id] = sock;
+	res = 0;
+
+exit:	
+	write_unlock_bh(&chan_lock);
+	
+	return res;
+}
+
+static void del_chan(struct pppox_sock *sock)
+{
+	write_lock_bh(&chan_lock);
+	clear_bit(sock->proto.pptp.src_addr.call_id,callid_bitmap);
+	callid_sock[sock->proto.pptp.src_addr.call_id] = NULL;
+	write_unlock_bh(&chan_lock);
+}
+
+static int pptp_xmit(struct ppp_channel *chan, struct sk_buff *skb)
+{
+	struct sock *sk = (struct sock *) chan->private;
+	struct pppox_sock *po = pppox_sk(sk);
+	struct pptp_opt *opt = &po->proto.pptp;
+	struct pptp_gre_header *hdr;
+	unsigned int header_len = sizeof(*hdr);
+	int err = 0;
+	int islcp;
+	int len;
+	unsigned char *data;
+	__u32 seq_recv;
+	
+	
+	struct rtable *rt;     			/* Route to the other host */
+	struct net_device *tdev;			/* Device to other host */
+	struct iphdr  *iph;			/* Our new IP header */
+	int    max_headroom;			/* The extra header space needed */
+
+	if (sk_pppox(po)->sk_state & PPPOX_DEAD)
+	    goto tx_error;
+
+	{
+		struct flowi fl = { .oif = 0,
+				    .nl_u = { .ip4_u =
+					      { .daddr = opt->dst_addr.sin_addr.s_addr,
+						.saddr = opt->src_addr.sin_addr.s_addr,
+						.tos = RT_TOS(0) } },
+				    .proto = IPPROTO_GRE };
+		if ((err=ip_route_output_key(&init_net,&rt, &fl))) {
+			goto tx_error;
+		}
+	}
+	tdev = rt->u.dst.dev;
+
+	max_headroom = LL_RESERVED_SPACE(tdev) + sizeof(*iph)+sizeof(*hdr)+2;
+
+	if (skb_headroom(skb) < max_headroom || skb_cloned(skb) || skb_shared(skb)) {
+		struct sk_buff *new_skb = skb_realloc_headroom(skb, max_headroom);
+		if (!new_skb) {
+			ip_rt_put(rt);
+			goto tx_error;
+		}
+		if (skb->sk)
+		skb_set_owner_w(new_skb, skb->sk);
+		kfree_skb(skb);
+		skb = new_skb;
+	}
+
+	data = skb->data;
+	islcp = ((data[0] << 8) + data[1])== PPP_LCP && 1 <= data[2] && data[2] <= 7;
+
+	/* compress protocol field */
+	if ((opt->ppp_flags & SC_COMP_PROT) && data[0] == 0 && !islcp)
+		skb_pull(skb,1);
+
+	/*
+		* Put in the address/control bytes if necessary
+		*/
+	if ((opt->ppp_flags & SC_COMP_AC) == 0 || islcp) {
+		data = skb_push(skb,2);
+		data[0] = PPP_ALLSTATIONS;
+		data[1] = PPP_UI;
+	}
+	
+	len = skb->len;
+  
+	seq_recv = opt->seq_recv;
+  
+	if (opt->ack_sent == seq_recv) header_len -= sizeof(hdr->ack);
+
+	// Push down and install GRE header
+	skb_push(skb,header_len);
+	hdr = (struct pptp_gre_header *)(skb->data);
+
+	hdr->flags       = PPTP_GRE_FLAG_K;
+	hdr->ver         = PPTP_GRE_VER;
+	hdr->protocol    = htons(PPTP_GRE_PROTO);
+	hdr->call_id     = htons(opt->dst_addr.call_id);
+
+	hdr->flags      |= PPTP_GRE_FLAG_S;
+	hdr->seq         = htonl(++opt->seq_sent);
+	#ifdef DEBUG
+	if (log_level >= 3 && opt->seq_sent <= log_packets)
+		printk(KERN_INFO"PPTP[%i]: send packet: seq=%i",opt->src_addr.call_id,opt->seq_sent);
+	#endif
+	if (opt->ack_sent != seq_recv)	{
+	/* send ack with this message */
+		hdr->ver |= PPTP_GRE_FLAG_A;
+		hdr->ack  = htonl(seq_recv);
+		opt->ack_sent = seq_recv;
+		#ifdef DEBUG
+		if (log_level >= 3 && opt->seq_sent <= log_packets)
+			printk(" ack=%i",seq_recv);
+		#endif
+	}
+	hdr->payload_len = htons(len);
+	#ifdef DEBUG
+	if (log_level >= 3 && opt->seq_sent <= log_packets)
+		printk("\n");
+	#endif
+
+	/*
+	 *	Push down and install the IP header.
+	 */
+
+	skb_reset_transport_header(skb);
+	skb_push(skb, sizeof(*iph));
+	skb_reset_network_header(skb);
+	memset(&(IPCB(skb)->opt), 0, sizeof(IPCB(skb)->opt));
+	IPCB(skb)->flags &= ~(IPSKB_XFRM_TUNNEL_SIZE | IPSKB_XFRM_TRANSFORMED | IPSKB_REROUTED);
+
+	iph =	ip_hdr(skb);
+	iph->version =	4;
+	iph->ihl =	sizeof(struct iphdr) >> 2;
+	if (ip_dont_fragment(sk, &rt->u.dst))
+		iph->frag_off	=	htons(IP_DF);
+	else
+		iph->frag_off	=	0;
+	iph->protocol	=	IPPROTO_GRE;
+	iph->tos		  =	0;
+	iph->daddr		=	rt->rt_dst;
+	iph->saddr		=	rt->rt_src;
+	iph->ttl      = dst_metric(&rt->u.dst, RTAX_HOPLIMIT);
+	iph->tot_len  = htons(skb->len);
+
+	skb_dst_drop(skb);
+	skb_dst_set(skb,&rt->u.dst);
+
+	nf_reset(skb);
+
+	skb->ip_summed = CHECKSUM_NONE;
+	ip_select_ident(iph, &rt->u.dst, NULL);
+	ip_send_check(iph);
+
+ 	ip_local_out(skb);
+
+tx_error:
+	return 1;
+}
+
+static int pptp_rcv_core(struct sock *sk,struct sk_buff *skb)
+{
+	struct pppox_sock *po = pppox_sk(sk);
+	struct pptp_opt *opt = &po->proto.pptp;
+	int headersize,payload_len,seq;
+	__u8 *payload;
+	struct pptp_gre_header *header;
+
+	if (!(sk->sk_state & PPPOX_CONNECTED)) {
+		if (sock_queue_rcv_skb(sk, skb))
+			goto drop;
+		return NET_RX_SUCCESS;
+	}
+	
+	header = (struct pptp_gre_header *)(skb->data);
+
+	/* test if acknowledgement present */
+	if (PPTP_GRE_IS_A(header->ver)){
+			__u32 ack = (PPTP_GRE_IS_S(header->flags))?
+					header->ack:header->seq; /* ack in different place if S = 0 */
+
+			ack = ntohl( ack);
+
+			if (ack > opt->ack_recv) opt->ack_recv = ack;
+			/* also handle sequence number wrap-around  */
+			if (WRAPPED(ack,opt->ack_recv)) opt->ack_recv = ack;
+	}
+
+	/* test if payload present */
+	if (!PPTP_GRE_IS_S(header->flags)){
+		goto drop;
+	}
+
+	headersize  = sizeof(*header);
+	payload_len = ntohs(header->payload_len);
+	seq         = ntohl(header->seq);
+
+	/* no ack present? */
+	if (!PPTP_GRE_IS_A(header->ver)) headersize -= sizeof(header->ack);
+	/* check for incomplete packet (length smaller than expected) */
+	if (skb->len - headersize < payload_len) {
+		#ifdef DEBUG
+		if (log_level >= 1)
+			printk(KERN_INFO"PPTP: discarding truncated packet (expected %d, got %d bytes)\n",
+						payload_len, skb->len - headersize);
+		#endif
+		goto drop;
+	}
+
+	payload = skb->data+headersize;
+	/* check for expected sequence number */
+	if ( seq < opt->seq_recv + 1 || WRAPPED(opt->seq_recv, seq) ) {
+		if ( (payload[0] == PPP_ALLSTATIONS) && (payload[1] == PPP_UI) &&
+		     (PPP_PROTOCOL(payload) == PPP_LCP) &&
+		     ((payload[4] == PPP_LCP_ECHOREQ) || (payload[4] == PPP_LCP_ECHOREP)) ) {
+			#ifdef DEBUG
+			if ( log_level >= 1)
+				printk(KERN_INFO"PPTP[%i]: allowing old LCP Echo packet %d (expecting %d)\n", opt->src_addr.call_id,
+							seq, opt->seq_recv + 1);
+			#endif
+			goto allow_packet;
+		}
+		#ifdef DEBUG
+		if ( log_level >= 1)
+			printk(KERN_INFO"PPTP[%i]: discarding duplicate or old packet %d (expecting %d)\n",opt->src_addr.call_id,
+							seq, opt->seq_recv + 1);
+		#endif
+	}else{
+		opt->seq_recv = seq;
+allow_packet:
+		#ifdef DEBUG
+		if ( log_level >= 3 && opt->seq_sent<=log_packets)
+			printk(KERN_INFO"PPTP[%i]: accepting packet %d size=%i (%02x %02x %02x %02x %02x %02x)\n",opt->src_addr.call_id, seq,payload_len,
+				*(payload +0),
+				*(payload +1),
+				*(payload +2),
+				*(payload +3),
+				*(payload +4),
+				*(payload +5));
+		#endif
+
+		skb_pull(skb,headersize);
+
+		if (payload[0] == PPP_ALLSTATIONS && payload[1] == PPP_UI) {
+			/* chop off address/control */
+			if (skb->len < 3)
+				goto drop;
+			skb_pull(skb,2);
+		}
+
+		if ((*skb->data) & 1){
+			/* protocol is compressed */
+			skb_push(skb, 1)[0] = 0;
+		}
+
+		skb->ip_summed = CHECKSUM_NONE;
+		skb_set_network_header(skb,skb->head-skb->data);
+		ppp_input(&po->chan,skb);
+
+		return NET_RX_SUCCESS;
+	}
+drop:
+	kfree_skb(skb);
+	return NET_RX_DROP;
+}
+
+static int pptp_rcv(struct sk_buff *skb)
+{
+	struct pppox_sock *po;
+	struct pptp_gre_header *header;
+	struct iphdr *iph;
+
+	if (skb->pkt_type != PACKET_HOST)
+		goto drop;
+
+	if (!pskb_may_pull(skb, 12))
+		goto drop;
+
+	iph = ip_hdr(skb);
+
+	header = (struct pptp_gre_header *)skb->data;
+
+	if (    /* version should be 1 */
+					((header->ver & 0x7F) != PPTP_GRE_VER) ||
+					/* PPTP-GRE protocol for PPTP */
+					(ntohs(header->protocol) != PPTP_GRE_PROTO)||
+					/* flag C should be clear   */
+					PPTP_GRE_IS_C(header->flags) ||
+					/* flag R should be clear   */
+					PPTP_GRE_IS_R(header->flags) ||
+					/* flag K should be set     */
+					(!PPTP_GRE_IS_K(header->flags)) ||
+					/* routing and recursion ctrl = 0  */
+					((header->flags&0xF) != 0)){
+			/* if invalid, discard this packet */
+		if (log_level >=1 )
+			printk(KERN_INFO"PPTP: Discarding GRE: %X %X %X %X %X %X\n",
+							header->ver&0x7F, ntohs(header->protocol),
+							PPTP_GRE_IS_C(header->flags),
+							PPTP_GRE_IS_R(header->flags),
+							PPTP_GRE_IS_K(header->flags),
+							header->flags & 0xF);
+		goto drop;
+	}
+
+
+	if ((po=lookup_chan(htons(header->call_id),iph->saddr))) {
+		skb_dst_drop(skb);
+		skb_dst_set(skb,NULL);
+		nf_reset(skb);
+		return sk_receive_skb(sk_pppox(po), skb, 0);
+	} else {
+		if (log_level >= 1)
+			printk(KERN_INFO"PPTP: Discarding packet from unknown call_id %i\n",htons(header->call_id));
+	}
+
+drop:
+	kfree_skb(skb);
+	return NET_RX_DROP;
+}
+
+static int pptp_bind(struct socket *sock,struct sockaddr *uservaddr,int sockaddr_len)
+{
+	struct sock *sk = sock->sk;
+	struct sockaddr_pppox *sp = (struct sockaddr_pppox *) uservaddr;
+	struct pppox_sock *po = pppox_sk(sk);
+	struct pptp_opt *opt = &po->proto.pptp;
+	int error = 0;
+
+	#ifdef DEBUG	
+	if (log_level >= 1)
+		printk(KERN_INFO"PPTP: bind: addr=%X call_id=%i\n",sp->sa_addr.pptp.sin_addr.s_addr,
+						sp->sa_addr.pptp.call_id);
+	#endif
+	lock_sock(sk);
+
+	opt->src_addr = sp->sa_addr.pptp;
+	if (add_chan(po)) {
+	  release_sock(sk);
+		error = -EBUSY;
+	}
+	#ifdef DEBUG
+	if (log_level >= 1)
+		printk(KERN_INFO"PPTP: using call_id %i\n",opt->src_addr.call_id);
+	#endif
+
+	release_sock(sk);
+	return error;
+}
+
+static int pptp_connect(struct socket *sock, struct sockaddr *uservaddr,
+		  int sockaddr_len, int flags)
+{
+	struct sock *sk = sock->sk;
+	struct sockaddr_pppox *sp = (struct sockaddr_pppox *) uservaddr;
+	struct pppox_sock *po = pppox_sk(sk);
+	struct pptp_opt *opt = &po->proto.pptp;
+	struct rtable *rt;     			/* Route to the other host */
+	int error=0;
+
+	if (sp->sa_protocol != PX_PROTO_PPTP)
+		return -EINVAL;
+	
+	#ifdef DEBUG
+	if (log_level >= 1)
+		printk(KERN_INFO"PPTP[%i]: connect: addr=%X call_id=%i\n",opt->src_addr.call_id,
+						sp->sa_addr.pptp.sin_addr.s_addr,sp->sa_addr.pptp.call_id);
+	#endif
+	
+	if (lookup_chan_dst(sp->sa_addr.pptp.call_id,sp->sa_addr.pptp.sin_addr.s_addr))
+		return -EALREADY;
+
+	lock_sock(sk);
+	/* Check for already bound sockets */
+	if (sk->sk_state & PPPOX_CONNECTED){
+		error = -EBUSY;
+		goto end;
+	}
+
+	/* Check for already disconnected sockets, on attempts to disconnect */
+	if (sk->sk_state & PPPOX_DEAD){
+		error = -EALREADY;
+		goto end;
+	}
+
+	if (!opt->src_addr.sin_addr.s_addr || !sp->sa_addr.pptp.sin_addr.s_addr){
+		error = -EINVAL;
+		goto end;
+	}
+
+	po->chan.private = sk;
+	po->chan.ops = &pptp_chan_ops;
+
+	{
+		struct flowi fl = {
+				    .nl_u = { .ip4_u =
+					      { .daddr = opt->dst_addr.sin_addr.s_addr,
+						.saddr = opt->src_addr.sin_addr.s_addr,
+						.tos = RT_CONN_FLAGS(sk) } },
+				    .proto = IPPROTO_GRE };
+		security_sk_classify_flow(sk, &fl);
+		if (ip_route_output_key(&init_net, &rt, &fl)) {
+			error = -EHOSTUNREACH;
+			goto end;
+		}
+		sk_setup_caps(sk, &rt->u.dst);
+	}
+	po->chan.mtu = dst_mtu(&rt->u.dst);
+	if (!po->chan.mtu) po->chan.mtu = PPP_MTU;
+	ip_rt_put(rt);
+	po->chan.mtu -= PPTP_HEADER_OVERHEAD;
+
+	po->chan.hdrlen = 2 + sizeof(struct pptp_gre_header);
+	error = ppp_register_channel(&po->chan);
+	if (error) {
+		printk(KERN_ERR "PPTP: failed to register PPP channel (%d)\n",error);
+		goto end;
+	}
+
+	opt->dst_addr = sp->sa_addr.pptp;
+	sk->sk_state = PPPOX_CONNECTED;
+
+ end:
+	release_sock(sk);
+	return error;
+}
+
+static int pptp_getname(struct socket *sock, struct sockaddr *uaddr,
+		  int *usockaddr_len, int peer)
+{
+	int len = sizeof(struct sockaddr_pppox);
+	struct sockaddr_pppox sp;
+
+	sp.sa_family	  = AF_PPPOX;
+	sp.sa_protocol  = PX_PROTO_PPTP;
+	sp.sa_addr.pptp = pppox_sk(sock->sk)->proto.pptp.src_addr;
+
+	memcpy(uaddr, &sp, len);
+
+	*usockaddr_len = len;
+
+	return 0;
+}
+
+static int pptp_release(struct socket *sock)
+{
+	struct sock *sk = sock->sk;
+	struct pppox_sock *po;
+	struct pptp_opt *opt;
+	int error = 0;
+
+	if (!sk)
+	    return 0;
+
+	lock_sock(sk);
+
+	if (sock_flag(sk, SOCK_DEAD)) {
+	    release_sock(sk);
+	    return -EBADF;
+	}
+		
+	po = pppox_sk(sk);
+	opt = &po->proto.pptp;
+	del_chan(po);
+
+	pppox_unbind_sock(sk);
+	sk->sk_state = PPPOX_DEAD;
+
+	#ifdef DEBUG
+	if (log_level >= 1)
+		printk(KERN_INFO"PPTP[%i]: release\n",opt->src_addr.call_id);
+	#endif
+
+	sock_orphan(sk);
+	sock->sk = NULL;
+
+	release_sock(sk);
+	sock_put(sk);
+
+	return error;
+}
+
+
+static struct proto pptp_sk_proto = {
+	.name	    = "PPTP",
+	.owner	  = THIS_MODULE,
+	.obj_size = sizeof(struct pppox_sock),
+};
+
+static struct proto_ops pptp_ops = {
+    .family		  = AF_PPPOX,
+    .owner		  = THIS_MODULE,
+    .release    = pptp_release,
+    .bind	   	  = pptp_bind,
+    .connect	  = pptp_connect,
+    .socketpair	= sock_no_socketpair,
+    .accept		  = sock_no_accept,
+    .getname		= pptp_getname,
+    .poll	    	= sock_no_poll,
+    .listen		  = sock_no_listen,
+    .shutdown		= sock_no_shutdown,
+    .setsockopt	= sock_no_setsockopt,
+    .getsockopt	= sock_no_getsockopt,
+    .sendmsg		= sock_no_sendmsg,
+    .recvmsg		= sock_no_recvmsg,
+    .mmap		    = sock_no_mmap,
+    .ioctl		  = pppox_ioctl,
+};
+
+static void pptp_sock_destruct(struct sock *sk)
+{
+    if (!(sk->sk_state & PPPOX_DEAD)) {
+	    del_chan(pppox_sk(sk));
+	    pppox_unbind_sock(sk);
+    }
+    skb_queue_purge(&sk->sk_receive_queue);
+}
+static int pptp_create(struct net *net, struct socket *sock)
+{
+	int error = -ENOMEM;
+	struct sock *sk;
+	struct pppox_sock *po;
+	struct pptp_opt *opt;
+
+	sk = sk_alloc(net,PF_PPPOX, GFP_KERNEL, &pptp_sk_proto);
+	if (!sk)
+		goto out;
+
+	sock_init_data(sock, sk);
+
+	sock->state = SS_UNCONNECTED;
+	sock->ops   = &pptp_ops;
+
+	sk->sk_backlog_rcv = pptp_rcv_core;
+	sk->sk_state	     = PPPOX_NONE;
+	sk->sk_type	       = SOCK_STREAM;
+	sk->sk_family	     = PF_PPPOX;
+	sk->sk_protocol	   = PX_PROTO_PPTP;
+	sk->sk_destruct	   = pptp_sock_destruct;
+
+	po = pppox_sk(sk);
+	opt = &po->proto.pptp;
+
+	opt->seq_sent = 0; opt->seq_recv = 0;
+	opt->ack_recv = 0; opt->ack_sent = 0;
+
+	error = 0;
+out:
+	return error;
+}
+
+static int pptp_ppp_ioctl(struct ppp_channel *chan, unsigned int cmd,
+			   unsigned long arg)
+{
+	struct sock *sk = (struct sock *) chan->private;
+	struct pppox_sock *po = pppox_sk(sk);
+	struct pptp_opt *opt = &po->proto.pptp;
+	void __user *argp = (void __user *)arg;
+	int __user *p = argp;
+	int err, val;
+
+	err = -EFAULT;
+	switch (cmd) {
+	case PPPIOCGFLAGS:
+		val = opt->ppp_flags;
+		if (put_user(val, p))
+			break;
+		err = 0;
+		break;
+	case PPPIOCSFLAGS:
+		if (get_user(val, p))
+			break;
+		opt->ppp_flags = val & ~SC_RCV_BITS;
+		err = 0;
+		break;
+	default:
+		err = -ENOTTY;
+	}
+
+	return err;
+}
+
+
+static struct pppox_proto pppox_pptp_proto = {
+    .create	= pptp_create,
+    .owner	= THIS_MODULE,
+};
+
+
+static struct gre_protocol gre_pptp_protocol = {
+	.handler	= pptp_rcv,
+	//.err_handler	=	pptp_err,
+};
+
+static int __init pptp_init_module(void)
+{
+	int err=0;
+	printk(KERN_INFO "PPTP driver version " PPTP_DRIVER_VERSION "\n");
+
+	if (gre_add_protocol(&gre_pptp_protocol, GREPROTO_PPTP) < 0) {
+		printk(KERN_INFO "PPTP: can't add protocol\n");
+		goto out;
+	}
+
+	err = proto_register(&pptp_sk_proto, 0);
+	if (err) {
+		printk(KERN_INFO "PPTP: can't register sk_proto\n");
+		goto out_inet_del_protocol;
+	}
+
+ 	err = register_pppox_proto(PX_PROTO_PPTP, &pppox_pptp_proto);
+	if (err) {
+		printk(KERN_INFO "PPTP: can't register pppox_proto\n");
+		goto out_unregister_sk_proto;
+	}
+	
+	
+	//assuming PAGESIZE is 4096 bytes
+	callid_bitmap = (unsigned long*)__get_free_pages(GFP_KERNEL,1);
+	memset(callid_bitmap,0,PAGE_SIZE << 1);
+
+	#if (BITS_PER_LONG == 32)
+	callid_sock = (struct pppox_sock **)__get_free_pages(GFP_KERNEL,6);
+	memset(callid_sock,0,PAGE_SIZE << 6);
+	#elif (BITS_PER_LONG == 64)
+	callid_sock = (struct pppox_sock **)__get_free_pages(GFP_KERNEL,7);
+	memset(callid_sock,0,PAGE_SIZE << 7);
+	#else
+	#error unknown size of LONG
+	#endif
+
+out:
+	return err;
+out_unregister_sk_proto:
+	proto_unregister(&pptp_sk_proto);
+out_inet_del_protocol:
+	gre_del_protocol(&gre_pptp_protocol, GREPROTO_PPTP);
+	return err;
+}
+
+static void __exit pptp_exit_module(void)
+{
+	unregister_pppox_proto(PX_PROTO_PPTP);
+	proto_unregister(&pptp_sk_proto);
+	gre_del_protocol(&gre_pptp_protocol, GREPROTO_PPTP);
+	if (callid_bitmap) free_pages((unsigned long)callid_bitmap,1);
+	if (callid_sock) {
+	    #if (BITS_PER_LONG == 32)
+	    free_pages((unsigned long)callid_sock,6);
+	    #elif (BITS_PER_LONG == 64)
+	    free_pages((unsigned long)callid_sock,7);
+	    #endif
+	}
+}
+
+module_init(pptp_init_module);
+module_exit(pptp_exit_module);
+
+MODULE_DESCRIPTION("Point-to-Point Tunneling Protocol for Linux");
+MODULE_AUTHOR("D. Kozlov (xeb@mail.ru)");
+MODULE_LICENSE("GPL");
+
+module_param(log_level,int,0);
+module_param(log_packets,int,0);
+MODULE_PARM_DESC(log_level,"Logging level (default=0)");
+
diff --git a/include/linux/if_pppox.h b/include/linux/if_pppox.h
index a6577af..455ff56 100644
--- a/include/linux/if_pppox.h
+++ b/include/linux/if_pppox.h
@@ -47,17 +47,27 @@ struct pppoe_addr{
 }; 
  
 /************************************************************************ 
+ * PPTP addressing definition 
+ */ 
+struct pptp_addr{
+       __u16           call_id;
+       struct in_addr  sin_addr;
+};
+
+/************************************************************************ 
  * Protocols supported by AF_PPPOX 
  */ 
 #define PX_PROTO_OE    0 /* Currently just PPPoE */
 #define PX_PROTO_OL2TP 1 /* Now L2TP also */
-#define PX_MAX_PROTO   2
+#define PX_PROTO_PPTP  2
+#define PX_MAX_PROTO   3
 
 struct sockaddr_pppox { 
        sa_family_t     sa_family;            /* address family, AF_PPPOX */ 
        unsigned int    sa_protocol;          /* protocol identifier */ 
        union{ 
                struct pppoe_addr       pppoe; 
+	       			 struct pptp_addr        pptp;
        }sa_addr; 
 }__attribute__ ((packed)); 
 
@@ -150,6 +160,13 @@ struct pppoe_opt {
 					     relayed to (PPPoE relaying) */
 };
 
+struct pptp_opt {
+	struct pptp_addr	src_addr;
+	struct pptp_addr	dst_addr;
+	__u32 ack_sent, ack_recv;
+	__u32 seq_sent, seq_recv;
+	int ppp_flags;
+};
 #include <net/sock.h>
 
 struct pppox_sock {
@@ -159,6 +176,7 @@ struct pppox_sock {
 	struct pppox_sock	*next;	  /* for hash table */
 	union {
 		struct pppoe_opt pppoe;
+		struct pptp_opt pptp;
 	} proto;
 	__be16			num;
 };


^ permalink raw reply related

* [PATCH 1/3] PPTP: PPP over IPv4 (Point-to-Point Tunneling Protocol)
From: xeb @ 2010-08-04 10:42 UTC (permalink / raw)
  To: netdev

This is patch 1/3 which contains gre demultiplexer driver source 
for demultiplexing gre packets with different gre version.
 
---
 include/net/gre.h |   18 ++++++
 net/ipv4/Kconfig  |    7 +++
 net/ipv4/Makefile |    1 +
 net/ipv4/gre.c    |  151 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 177 insertions(+), 0 deletions(-)

diff --git a/include/net/gre.h b/include/net/gre.h
new file mode 100644
index 0000000..31a0f76
--- /dev/null
+++ b/include/net/gre.h
@@ -0,0 +1,18 @@
+#ifndef __LINUX_GRE_H
+#define __LINUX_GRE_H
+
+#include <linux/skbuff.h>
+
+#define GREPROTO_CISCO		0
+#define GREPROTO_PPTP		1
+#define GREPROTO_MAX		2
+
+struct gre_protocol {
+	int		(*handler)(struct sk_buff *skb);
+	void	(*err_handler)(struct sk_buff *skb, u32 info);
+};
+
+int gre_add_protocol(const struct gre_protocol *proto, u8 version);
+int gre_del_protocol(const struct gre_protocol *proto, u8 version);
+
+#endif
diff --git a/net/ipv4/Kconfig b/net/ipv4/Kconfig
index 7c3a7d1..7458bda 100644
--- a/net/ipv4/Kconfig
+++ b/net/ipv4/Kconfig
@@ -215,8 +215,15 @@ config NET_IPIP
 	  be inserted in and removed from the running kernel whenever you
 	  want). Most people won't need this and can say N.
 
+config NET_IPGRE_DEMUX
+	tristate "IP: GRE demultiplexer"
+	help
+	 This is helper module to demultiplex GRE packets on GRE version field criteria.
+	 Required by ip_gre and pptp modules.
+
 config NET_IPGRE
 	tristate "IP: GRE tunnels over IP"
+	depends on NET_IPGRE_DEMUX
 	help
 	  Tunneling means encapsulating data of one protocol type within
 	  another protocol and sending it over a channel that understands the
diff --git a/net/ipv4/Makefile b/net/ipv4/Makefile
index 80ff87c..4978d22 100644
--- a/net/ipv4/Makefile
+++ b/net/ipv4/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_PROC_FS) += proc.o
 obj-$(CONFIG_IP_MULTIPLE_TABLES) += fib_rules.o
 obj-$(CONFIG_IP_MROUTE) += ipmr.o
 obj-$(CONFIG_NET_IPIP) += ipip.o
+obj-$(CONFIG_NET_IPGRE_DEMUX) += gre.o
 obj-$(CONFIG_NET_IPGRE) += ip_gre.o
 obj-$(CONFIG_SYN_COOKIES) += syncookies.o
 obj-$(CONFIG_INET_AH) += ah4.o
diff --git a/net/ipv4/gre.c b/net/ipv4/gre.c
new file mode 100644
index 0000000..993f3cf
--- /dev/null
+++ b/net/ipv4/gre.c
@@ -0,0 +1,151 @@
+/*
+ *	GRE over IPv4 demultiplexer driver
+ *
+ *	Authors: Dmitry Kozlov (xeb@mail.ru)
+ *
+ *	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.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/kmod.h>
+#include <linux/skbuff.h>
+#include <linux/in.h>
+#include <linux/netdevice.h>
+#include <linux/version.h>
+#include <linux/spinlock.h>
+#include <net/protocol.h>
+#include <net/gre.h>
+
+
+const struct gre_protocol *gre_proto[GREPROTO_MAX] ____cacheline_aligned_in_smp;
+static DEFINE_RWLOCK(gre_proto_lock);
+
+int gre_add_protocol(const struct gre_protocol *proto, u8 version)
+{
+	int ret;
+
+	if (version >= GREPROTO_MAX)
+		return -1;
+	
+	write_lock_bh(&gre_proto_lock);
+	if (gre_proto[version]) {
+		ret = -1;
+	} else {
+		gre_proto[version]=proto;
+		ret = 0;
+	}
+	write_unlock_bh(&gre_proto_lock);
+
+	return ret;
+}
+int gre_del_protocol(const struct gre_protocol *proto, u8 version)
+{
+	int ret;
+
+	if (version >= GREPROTO_MAX)
+		return -1;
+
+	write_lock_bh(&gre_proto_lock);
+	if (gre_proto[version] == proto) {
+		gre_proto[version] = NULL;
+		ret = 0;
+	} else {
+		ret = -1;
+	}
+	write_unlock_bh(&gre_proto_lock);
+
+	return ret;
+}
+static int gre_rcv(struct sk_buff *skb)
+{
+	u8 ver;
+	int ret;
+
+	if (!pskb_may_pull(skb, 12))
+		goto drop_nolock;
+
+	ver = skb->data[1]&0x7f;
+	if (ver >= GREPROTO_MAX)
+		goto drop_nolock;
+	
+	read_lock(&gre_proto_lock);
+	if (!gre_proto[ver] || !gre_proto[ver]->handler)
+		goto drop;
+
+	ret = gre_proto[ver]->handler(skb);
+	read_unlock(&gre_proto_lock);
+
+	return ret;
+
+drop:
+	read_unlock(&gre_proto_lock);
+drop_nolock:
+	kfree_skb(skb);
+	return NET_RX_DROP;
+}
+static void gre_err(struct sk_buff *skb, u32 info)
+{
+	u8 ver;
+
+	printk("err\n");
+
+	if (!pskb_may_pull(skb, 12))
+		goto drop_nolock;
+
+	ver=skb->data[1];
+	if (ver>=GREPROTO_MAX)
+		goto drop_nolock;
+		
+	read_lock(&gre_proto_lock);
+	if (!gre_proto[ver] || !gre_proto[ver]->err_handler)
+		goto drop;
+
+	gre_proto[ver]->err_handler(skb,info);
+	read_unlock(&gre_proto_lock);
+
+	return;
+
+drop:
+	read_unlock(&gre_proto_lock);
+drop_nolock:
+	kfree_skb(skb);
+}
+
+
+static struct net_protocol net_gre_protocol = {
+	.handler	= gre_rcv,
+	.err_handler	=	gre_err,
+//	.netns_ok=1,
+};
+
+static int __init gre_init(void)
+{
+	printk(KERN_INFO "GRE over IPv4 demultiplexor driver");
+	
+	if (inet_add_protocol(&net_gre_protocol, IPPROTO_GRE) < 0) {
+		printk(KERN_INFO "gre: can't add protocol\n");
+		return -EAGAIN;
+	}
+
+	return 0;
+}
+
+static void __exit gre_exit(void)
+{
+	inet_del_protocol(&net_gre_protocol, IPPROTO_GRE);
+}
+
+module_init(gre_init);
+module_exit(gre_exit);
+
+MODULE_DESCRIPTION("GRE over IPv4 demultiplexer driver");
+MODULE_AUTHOR("D. Kozlov (xeb@mail.ru)");
+MODULE_LICENSE("GPL");
+EXPORT_SYMBOL_GPL(gre_add_protocol);
+EXPORT_SYMBOL_GPL(gre_del_protocol);


^ permalink raw reply related

* Re: [PATCH repost] sched: export sched_set/getaffinity to modules
From: Peter Zijlstra @ 2010-08-04 10:45 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Oleg Nesterov, Sridhar Samudrala, Tejun Heo, Ingo Molnar, netdev,
	lkml, kvm@vger.kernel.org, Andrew Morton, Dmitri Vorobiev,
	Jiri Kosina, Thomas Gleixner, Andi Kleen
In-Reply-To: <20100727045534.GA4316@redhat.com>

On Tue, 2010-07-27 at 07:55 +0300, Michael S. Tsirkin wrote:

> Peter, could you please indicate whether you think this is the way to
> go, too?

I really dislike it, as you indicated, you now want priority too..

It seems the problem is that we normally don't consider work done by
kernel threads for user processes part of that process.

I'm not sure what work you're doing, but I'm pretty sure there's similar
things already in the kernel -- think about the work done by encryption
threads for encrypted sockets and stuff.

If you want proper containment of work caused by a process, I'd suggest
you start by looking at curing the general problem, instead of special
casing this one case.

^ permalink raw reply

* [PATCH 3/3] PPTP: PPP over IPv4 (Point-to-Point Tunneling Protocol)
From: xeb @ 2010-08-04 10:48 UTC (permalink / raw)
  To: netdev

This is patch 3/3 which contains MAINTAINERS file modification.

---
 MAINTAINERS |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 02f75fc..313d829 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6450,6 +6450,20 @@ M:	"Maciej W. Rozycki" <macro@linux-mips.org>
 S:	Maintained
 F:	drivers/serial/zs.*
 
+GRE DEMULTIPLEXER DRIVER
+M:	Dmitry Kozlov <xeb@mail.ru>
+L:	linux-net@vger.kernel.org
+S:	Maintained
+F:	net/ipv4/gre.c
+F:	include/net/gre.h
+
+PPTP DRIVER
+M:	Dmitry Kozlov <xeb@mail.ru>
+L:	netdev@vger.kernel.org
+S:	Maintained
+F:	drivers/net/pptp.c
+W:	http://accel-pptp.sourceforge.net/
+
 THE REST
 M:	Linus Torvalds <torvalds@linux-foundation.org>
 L:	linux-kernel@vger.kernel.org

^ permalink raw reply related

* [PATCH v4 1/2] core: Factor out flow calculation from get_rps_cpu
From: Krishna Kumar @ 2010-08-04 10:55 UTC (permalink / raw)
  To: davem, arnd; +Cc: mst, netdev, xiaosuo, bhutchings, Krishna Kumar, therbert

From: Krishna Kumar <krkumar2@in.ibm.com>

Factor out flow calculation code from get_rps_cpu, since other
functions can use the same code.

Revisions:

v2 - Ben: Separate flow calcuation out and use in select queue.
v3 - Arnd: Don't re-implement MIN.
v4 - Changli: skb->data points to ethernet header in macvtap, and
	make a fast path.

Tested macvtap with this patch.

Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
---
 include/linux/skbuff.h |   11 ++++
 net/core/dev.c         |  107 ++++++++++++++++++++++-----------------
 2 files changed, 74 insertions(+), 44 deletions(-)

diff -ruNp org/include/linux/skbuff.h new/include/linux/skbuff.h
--- org/include/linux/skbuff.h	2010-08-04 09:07:05.000000000 +0530
+++ new/include/linux/skbuff.h	2010-08-04 16:19:30.000000000 +0530
@@ -558,6 +558,17 @@ extern unsigned int   skb_find_text(stru
 				    unsigned int to, struct ts_config *config,
 				    struct ts_state *state);
 
+extern __u32 __skb_get_rxhash(struct sk_buff *skb);
+static inline __u32 skb_get_rxhash(struct sk_buff *skb)
+{
+	__u32 rxhash = skb->rxhash;
+
+	if (!rxhash)
+		rxhash = __skb_get_rxhash(skb);
+
+	return rxhash;
+}
+
 #ifdef NET_SKBUFF_DATA_USES_OFFSET
 static inline unsigned char *skb_end_pointer(const struct sk_buff *skb)
 {
diff -ruNp org/net/core/dev.c new/net/core/dev.c
--- org/net/core/dev.c	2010-08-04 09:07:05.000000000 +0530
+++ new/net/core/dev.c	2010-08-04 16:19:30.000000000 +0530
@@ -2259,69 +2259,41 @@ static inline void ____napi_schedule(str
 	__raise_softirq_irqoff(NET_RX_SOFTIRQ);
 }
 
-#ifdef CONFIG_RPS
-
-/* One global table that all flow-based protocols share. */
-struct rps_sock_flow_table *rps_sock_flow_table __read_mostly;
-EXPORT_SYMBOL(rps_sock_flow_table);
-
 /*
- * get_rps_cpu is called from netif_receive_skb and returns the target
- * CPU from the RPS map of the receiving queue for a given skb.
- * rcu_read_lock must be held on entry.
+ * __skb_get_rxhash: calculate a flow hash based on src/dst addresses
+ * and src/dst port numbers. On success, returns a non-zero hash number,
+ * otherwise 0.
  */
-static int get_rps_cpu(struct net_device *dev, struct sk_buff *skb,
-		       struct rps_dev_flow **rflowp)
+__u32 __skb_get_rxhash(struct sk_buff *skb)
 {
+	int nhoff, hash = 0;
 	struct ipv6hdr *ip6;
 	struct iphdr *ip;
-	struct netdev_rx_queue *rxqueue;
-	struct rps_map *map;
-	struct rps_dev_flow_table *flow_table;
-	struct rps_sock_flow_table *sock_flow_table;
-	int cpu = -1;
 	u8 ip_proto;
-	u16 tcpu;
 	u32 addr1, addr2, ihl;
 	union {
 		u32 v32;
 		u16 v16[2];
 	} ports;
 
-	if (skb_rx_queue_recorded(skb)) {
-		u16 index = skb_get_rx_queue(skb);
-		if (unlikely(index >= dev->num_rx_queues)) {
-			WARN_ONCE(dev->num_rx_queues > 1, "%s received packet "
-				"on queue %u, but number of RX queues is %u\n",
-				dev->name, index, dev->num_rx_queues);
-			goto done;
-		}
-		rxqueue = dev->_rx + index;
-	} else
-		rxqueue = dev->_rx;
-
-	if (!rxqueue->rps_map && !rxqueue->rps_flow_table)
-		goto done;
-
-	if (skb->rxhash)
-		goto got_hash; /* Skip hash computation on packet header */
+	nhoff = skb_network_offset(skb);
 
 	switch (skb->protocol) {
 	case __constant_htons(ETH_P_IP):
-		if (!pskb_may_pull(skb, sizeof(*ip)))
+		if (!pskb_may_pull(skb, sizeof(*ip) + nhoff))
 			goto done;
 
-		ip = (struct iphdr *) skb->data;
+		ip = (struct iphdr *) skb->data + nhoff;
 		ip_proto = ip->protocol;
 		addr1 = (__force u32) ip->saddr;
 		addr2 = (__force u32) ip->daddr;
 		ihl = ip->ihl;
 		break;
 	case __constant_htons(ETH_P_IPV6):
-		if (!pskb_may_pull(skb, sizeof(*ip6)))
+		if (!pskb_may_pull(skb, sizeof(*ip6) + nhoff))
 			goto done;
 
-		ip6 = (struct ipv6hdr *) skb->data;
+		ip6 = (struct ipv6hdr *) skb->data + nhoff;
 		ip_proto = ip6->nexthdr;
 		addr1 = (__force u32) ip6->saddr.s6_addr32[3];
 		addr2 = (__force u32) ip6->daddr.s6_addr32[3];
@@ -2330,6 +2302,7 @@ static int get_rps_cpu(struct net_device
 	default:
 		goto done;
 	}
+
 	switch (ip_proto) {
 	case IPPROTO_TCP:
 	case IPPROTO_UDP:
@@ -2338,8 +2311,9 @@ static int get_rps_cpu(struct net_device
 	case IPPROTO_AH:
 	case IPPROTO_SCTP:
 	case IPPROTO_UDPLITE:
-		if (pskb_may_pull(skb, (ihl * 4) + 4)) {
-			ports.v32 = * (__force u32 *) (skb->data + (ihl * 4));
+		if (pskb_may_pull(skb, (ihl * 4) + 4 + nhoff)) {
+			ports.v32 = * (__force u32 *) (skb->data + nhoff +
+						       (ihl * 4));
 			if (ports.v16[1] < ports.v16[0])
 				swap(ports.v16[0], ports.v16[1]);
 			break;
@@ -2352,11 +2326,56 @@ static int get_rps_cpu(struct net_device
 	/* get a consistent hash (same value on both flow directions) */
 	if (addr2 < addr1)
 		swap(addr1, addr2);
-	skb->rxhash = jhash_3words(addr1, addr2, ports.v32, hashrnd);
-	if (!skb->rxhash)
-		skb->rxhash = 1;
 
-got_hash:
+	hash = jhash_3words(addr1, addr2, ports.v32, hashrnd);
+	if (!hash)
+		hash = 1;
+
+done:
+	return hash;
+}
+EXPORT_SYMBOL(__skb_get_rxhash);
+
+#ifdef CONFIG_RPS
+
+/* One global table that all flow-based protocols share. */
+struct rps_sock_flow_table *rps_sock_flow_table __read_mostly;
+EXPORT_SYMBOL(rps_sock_flow_table);
+
+/*
+ * get_rps_cpu is called from netif_receive_skb and returns the target
+ * CPU from the RPS map of the receiving queue for a given skb.
+ * rcu_read_lock must be held on entry.
+ */
+static int get_rps_cpu(struct net_device *dev, struct sk_buff *skb,
+		       struct rps_dev_flow **rflowp)
+{
+	struct netdev_rx_queue *rxqueue;
+	struct rps_map *map;
+	struct rps_dev_flow_table *flow_table;
+	struct rps_sock_flow_table *sock_flow_table;
+	int cpu = -1;
+	u16 tcpu;
+
+	if (skb_rx_queue_recorded(skb)) {
+		u16 index = skb_get_rx_queue(skb);
+		if (unlikely(index >= dev->num_rx_queues)) {
+			WARN_ONCE(dev->num_rx_queues > 1, "%s received packet "
+				"on queue %u, but number of RX queues is %u\n",
+				dev->name, index, dev->num_rx_queues);
+			goto done;
+		}
+		rxqueue = dev->_rx + index;
+	} else
+		rxqueue = dev->_rx;
+
+	if (!rxqueue->rps_map && !rxqueue->rps_flow_table)
+		goto done;
+
+	skb->rxhash = skb_get_rxhash(skb);
+	if (skb->rxhash < 0)
+		goto done;
+
 	flow_table = rcu_dereference(rxqueue->rps_flow_table);
 	sock_flow_table = rcu_dereference(rps_sock_flow_table);
 	if (flow_table && sock_flow_table) {

^ permalink raw reply

* [PATCH v4 2/2] macvtap: Implement multiqueue for macvtap driver
From: Krishna Kumar @ 2010-08-04 10:55 UTC (permalink / raw)
  To: davem, arnd; +Cc: mst, netdev, xiaosuo, bhutchings, Krishna Kumar, therbert
In-Reply-To: <20100804105515.22212.41598.sendpatchset@krkumar2.in.ibm.com>

From: Krishna Kumar <krkumar2@in.ibm.com>

Implement multiqueue facility for macvtap driver. The idea is that
a macvtap device can be opened multiple times and the fd's can be
used to register eg, as backend for vhost.

Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
---
 drivers/net/macvtap.c      |   89 ++++++++++++++++++++++++++++-------
 include/linux/if_macvlan.h |    9 +++
 2 files changed, 80 insertions(+), 18 deletions(-)

diff -ruNp org/drivers/net/macvtap.c new/drivers/net/macvtap.c
--- org/drivers/net/macvtap.c	2010-07-28 15:10:10.000000000 +0530
+++ new/drivers/net/macvtap.c	2010-08-04 16:19:32.000000000 +0530
@@ -84,26 +84,45 @@ static const struct proto_ops macvtap_so
 static DEFINE_SPINLOCK(macvtap_lock);
 
 /*
- * Choose the next free queue, for now there is only one
+ * get_slot: return a [unused/occupied] slot in vlan->taps[]:
+ *	- if 'q' is NULL, return the first empty slot;
+ *	- otherwise, return the slot this pointer occupies.
  */
+static int get_slot(struct macvlan_dev *vlan, struct macvtap_queue *q)
+{
+	int i;
+
+	for (i = 0; i < MAX_MACVTAP_QUEUES; i++) {
+		if (rcu_dereference(vlan->taps[i]) == q)
+			return i;
+	}
+
+	/* Should never happen */
+	BUG_ON(1);
+}
+
 static int macvtap_set_queue(struct net_device *dev, struct file *file,
 				struct macvtap_queue *q)
 {
 	struct macvlan_dev *vlan = netdev_priv(dev);
+	int index;
 	int err = -EBUSY;
 
 	spin_lock(&macvtap_lock);
-	if (rcu_dereference(vlan->tap))
+	if (vlan->numvtaps == MAX_MACVTAP_QUEUES)
 		goto out;
 
 	err = 0;
+	index = get_slot(vlan, NULL);
 	rcu_assign_pointer(q->vlan, vlan);
-	rcu_assign_pointer(vlan->tap, q);
+	rcu_assign_pointer(vlan->taps[index], q);
 	sock_hold(&q->sk);
 
 	q->file = file;
 	file->private_data = q;
 
+	vlan->numvtaps++;
+
 out:
 	spin_unlock(&macvtap_lock);
 	return err;
@@ -124,9 +143,12 @@ static void macvtap_put_queue(struct mac
 	spin_lock(&macvtap_lock);
 	vlan = rcu_dereference(q->vlan);
 	if (vlan) {
-		rcu_assign_pointer(vlan->tap, NULL);
+		int index = get_slot(vlan, q);
+
+		rcu_assign_pointer(vlan->taps[index], NULL);
 		rcu_assign_pointer(q->vlan, NULL);
 		sock_put(&q->sk);
+		--vlan->numvtaps;
 	}
 
 	spin_unlock(&macvtap_lock);
@@ -136,39 +158,72 @@ static void macvtap_put_queue(struct mac
 }
 
 /*
- * Since we only support one queue, just dereference the pointer.
+ * Select a queue based on the rxq of the device on which this packet
+ * arrived. If the incoming device is not mq, calculate a flow hash to
+ * select a queue. vlan->numvtaps is cached in case it reduces during
+ * the execution of this function.
  */
 static struct macvtap_queue *macvtap_get_queue(struct net_device *dev,
 					       struct sk_buff *skb)
 {
 	struct macvlan_dev *vlan = netdev_priv(dev);
+	struct macvtap_queue *tap = NULL;
+	int numvtaps = vlan->numvtaps;
+	__u32 rxq;
+
+	if (!numvtaps)
+		goto out;
+
+	if (likely(skb_rx_queue_recorded(skb))) {
+		rxq = skb_get_rx_queue(skb);
+
+		while (unlikely(rxq >= numvtaps))
+			rxq -= numvtaps;
 
-	return rcu_dereference(vlan->tap);
+		tap = rcu_dereference(vlan->taps[rxq]);
+		if (tap)
+			goto out;
+	}
+
+	rxq = skb_get_rxhash(skb);
+	if (!rxq)
+		rxq = smp_processor_id();
+
+	tap = rcu_dereference(vlan->taps[rxq & (numvtaps - 1)]);
+
+out:
+	return tap;
 }
 
 /*
  * The net_device is going away, give up the reference
- * that it holds on the queue (all the queues one day)
- * and safely set the pointer from the queues to NULL.
+ * that it holds on all queues and safely set the pointer
+ * from the queues to NULL.
  */
 static void macvtap_del_queues(struct net_device *dev)
 {
 	struct macvlan_dev *vlan = netdev_priv(dev);
-	struct macvtap_queue *q;
+	struct macvtap_queue *q, *qlist[MAX_MACVTAP_QUEUES];
+	int i, j = 0;
 
+	/* macvtap_put_queue can free some slots, so go through all slots */
 	spin_lock(&macvtap_lock);
-	q = rcu_dereference(vlan->tap);
-	if (!q) {
-		spin_unlock(&macvtap_lock);
-		return;
+	for (i = 0; i < MAX_MACVTAP_QUEUES && vlan->numvtaps; i++) {
+		q = rcu_dereference(vlan->taps[i]);
+		if (q) {
+			qlist[j++] = q;
+			rcu_assign_pointer(vlan->taps[i], NULL);
+			rcu_assign_pointer(q->vlan, NULL);
+			vlan->numvtaps--;
+		}
 	}
-
-	rcu_assign_pointer(vlan->tap, NULL);
-	rcu_assign_pointer(q->vlan, NULL);
+	BUG_ON(vlan->numvtaps != 0);
 	spin_unlock(&macvtap_lock);
 
 	synchronize_rcu();
-	sock_put(&q->sk);
+
+	for (--j; j >= 0; j--)
+		sock_put(&qlist[j]->sk);
 }
 
 /*
diff -ruNp org/include/linux/if_macvlan.h new/include/linux/if_macvlan.h
--- org/include/linux/if_macvlan.h	2010-07-28 15:10:10.000000000 +0530
+++ new/include/linux/if_macvlan.h	2010-08-04 16:19:32.000000000 +0530
@@ -40,6 +40,12 @@ struct macvlan_rx_stats {
 	unsigned long		rx_errors;
 };
 
+/*
+ * Maximum times a macvtap device can be opened. This can be used to
+ * configure the number of receive queue, e.g. for multiqueue virtio.
+ */
+#define MAX_MACVTAP_QUEUES	(NR_CPUS < 16 ? NR_CPUS : 16)
+
 struct macvlan_dev {
 	struct net_device	*dev;
 	struct list_head	list;
@@ -50,7 +56,8 @@ struct macvlan_dev {
 	enum macvlan_mode	mode;
 	int (*receive)(struct sk_buff *skb);
 	int (*forward)(struct net_device *dev, struct sk_buff *skb);
-	struct macvtap_queue	*tap;
+	struct macvtap_queue	*taps[MAX_MACVTAP_QUEUES];
+	int			numvtaps;
 };
 
 static inline void macvlan_count_rx(const struct macvlan_dev *vlan,

^ permalink raw reply

* Re: [PATCH v4 1/2] core: Factor out flow calculation from get_rps_cpu
From: Changli Gao @ 2010-08-04 11:22 UTC (permalink / raw)
  To: Krishna Kumar; +Cc: davem, arnd, mst, netdev, bhutchings, therbert
In-Reply-To: <20100804105515.22212.41598.sendpatchset@krkumar2.in.ibm.com>

On Wed, Aug 4, 2010 at 6:55 PM, Krishna Kumar <krkumar2@in.ibm.com> wrote:
> From: Krishna Kumar <krkumar2@in.ibm.com>
>
> Factor out flow calculation code from get_rps_cpu, since other
> functions can use the same code.
>
> Revisions:
>
> v2 - Ben: Separate flow calcuation out and use in select queue.
> v3 - Arnd: Don't re-implement MIN.
> v4 - Changli: skb->data points to ethernet header in macvtap, and
>        make a fast path.
>
> Tested macvtap with this patch.
>
> Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
> ---
>  include/linux/skbuff.h |   11 ++++
>  net/core/dev.c         |  107 ++++++++++++++++++++++-----------------
>  2 files changed, 74 insertions(+), 44 deletions(-)
>
> diff -ruNp org/include/linux/skbuff.h new/include/linux/skbuff.h
> --- org/include/linux/skbuff.h  2010-08-04 09:07:05.000000000 +0530
> +++ new/include/linux/skbuff.h  2010-08-04 16:19:30.000000000 +0530
> @@ -558,6 +558,17 @@ extern unsigned int   skb_find_text(stru
>                                    unsigned int to, struct ts_config *config,
>                                    struct ts_state *state);
>
> +extern __u32 __skb_get_rxhash(struct sk_buff *skb);
> +static inline __u32 skb_get_rxhash(struct sk_buff *skb)
> +{
> +       __u32 rxhash = skb->rxhash;
> +
> +       if (!rxhash)
> +               rxhash = __skb_get_rxhash(skb);
> +
> +       return rxhash;
> +}
> +
>  #ifdef NET_SKBUFF_DATA_USES_OFFSET
>  static inline unsigned char *skb_end_pointer(const struct sk_buff *skb)
>  {
> diff -ruNp org/net/core/dev.c new/net/core/dev.c
> --- org/net/core/dev.c  2010-08-04 09:07:05.000000000 +0530
> +++ new/net/core/dev.c  2010-08-04 16:19:30.000000000 +0530
> @@ -2259,69 +2259,41 @@ static inline void ____napi_schedule(str
>        __raise_softirq_irqoff(NET_RX_SOFTIRQ);
>  }
>
> -#ifdef CONFIG_RPS
> -
> -/* One global table that all flow-based protocols share. */
> -struct rps_sock_flow_table *rps_sock_flow_table __read_mostly;
> -EXPORT_SYMBOL(rps_sock_flow_table);
> -
>  /*
> - * get_rps_cpu is called from netif_receive_skb and returns the target
> - * CPU from the RPS map of the receiving queue for a given skb.
> - * rcu_read_lock must be held on entry.
> + * __skb_get_rxhash: calculate a flow hash based on src/dst addresses
> + * and src/dst port numbers. On success, returns a non-zero hash number,
> + * otherwise 0.
>  */
> -static int get_rps_cpu(struct net_device *dev, struct sk_buff *skb,
> -                      struct rps_dev_flow **rflowp)
> +__u32 __skb_get_rxhash(struct sk_buff *skb)
>  {
> +       int nhoff, hash = 0;
>        struct ipv6hdr *ip6;
>        struct iphdr *ip;
> -       struct netdev_rx_queue *rxqueue;
> -       struct rps_map *map;
> -       struct rps_dev_flow_table *flow_table;
> -       struct rps_sock_flow_table *sock_flow_table;
> -       int cpu = -1;
>        u8 ip_proto;
> -       u16 tcpu;
>        u32 addr1, addr2, ihl;
>        union {
>                u32 v32;
>                u16 v16[2];
>        } ports;
>
> -       if (skb_rx_queue_recorded(skb)) {
> -               u16 index = skb_get_rx_queue(skb);
> -               if (unlikely(index >= dev->num_rx_queues)) {
> -                       WARN_ONCE(dev->num_rx_queues > 1, "%s received packet "
> -                               "on queue %u, but number of RX queues is %u\n",
> -                               dev->name, index, dev->num_rx_queues);
> -                       goto done;
> -               }
> -               rxqueue = dev->_rx + index;
> -       } else
> -               rxqueue = dev->_rx;
> -
> -       if (!rxqueue->rps_map && !rxqueue->rps_flow_table)
> -               goto done;
> -
> -       if (skb->rxhash)
> -               goto got_hash; /* Skip hash computation on packet header */
> +       nhoff = skb_network_offset(skb);
>
>        switch (skb->protocol) {
>        case __constant_htons(ETH_P_IP):
> -               if (!pskb_may_pull(skb, sizeof(*ip)))
> +               if (!pskb_may_pull(skb, sizeof(*ip) + nhoff))
>                        goto done;
>
> -               ip = (struct iphdr *) skb->data;
> +               ip = (struct iphdr *) skb->data + nhoff;
>                ip_proto = ip->protocol;
>                addr1 = (__force u32) ip->saddr;
>                addr2 = (__force u32) ip->daddr;
>                ihl = ip->ihl;
>                break;
>        case __constant_htons(ETH_P_IPV6):
> -               if (!pskb_may_pull(skb, sizeof(*ip6)))
> +               if (!pskb_may_pull(skb, sizeof(*ip6) + nhoff))
>                        goto done;
>
> -               ip6 = (struct ipv6hdr *) skb->data;
> +               ip6 = (struct ipv6hdr *) skb->data + nhoff;
>                ip_proto = ip6->nexthdr;
>                addr1 = (__force u32) ip6->saddr.s6_addr32[3];
>                addr2 = (__force u32) ip6->daddr.s6_addr32[3];
> @@ -2330,6 +2302,7 @@ static int get_rps_cpu(struct net_device
>        default:
>                goto done;
>        }
> +
>        switch (ip_proto) {
>        case IPPROTO_TCP:
>        case IPPROTO_UDP:
> @@ -2338,8 +2311,9 @@ static int get_rps_cpu(struct net_device
>        case IPPROTO_AH:
>        case IPPROTO_SCTP:
>        case IPPROTO_UDPLITE:
> -               if (pskb_may_pull(skb, (ihl * 4) + 4)) {
> -                       ports.v32 = * (__force u32 *) (skb->data + (ihl * 4));
> +               if (pskb_may_pull(skb, (ihl * 4) + 4 + nhoff)) {
> +                       ports.v32 = * (__force u32 *) (skb->data + nhoff +
> +                                                      (ihl * 4));
>                        if (ports.v16[1] < ports.v16[0])
>                                swap(ports.v16[0], ports.v16[1]);
>                        break;
> @@ -2352,11 +2326,56 @@ static int get_rps_cpu(struct net_device
>        /* get a consistent hash (same value on both flow directions) */
>        if (addr2 < addr1)
>                swap(addr1, addr2);
> -       skb->rxhash = jhash_3words(addr1, addr2, ports.v32, hashrnd);
> -       if (!skb->rxhash)
> -               skb->rxhash = 1;
>
> -got_hash:
> +       hash = jhash_3words(addr1, addr2, ports.v32, hashrnd);
> +       if (!hash)
> +               hash = 1;
> +
> +done:
> +       return hash;
> +}
> +EXPORT_SYMBOL(__skb_get_rxhash);
> +
> +#ifdef CONFIG_RPS
> +
> +/* One global table that all flow-based protocols share. */
> +struct rps_sock_flow_table *rps_sock_flow_table __read_mostly;
> +EXPORT_SYMBOL(rps_sock_flow_table);
> +
> +/*
> + * get_rps_cpu is called from netif_receive_skb and returns the target
> + * CPU from the RPS map of the receiving queue for a given skb.
> + * rcu_read_lock must be held on entry.
> + */
> +static int get_rps_cpu(struct net_device *dev, struct sk_buff *skb,
> +                      struct rps_dev_flow **rflowp)
> +{
> +       struct netdev_rx_queue *rxqueue;
> +       struct rps_map *map;
> +       struct rps_dev_flow_table *flow_table;
> +       struct rps_sock_flow_table *sock_flow_table;
> +       int cpu = -1;
> +       u16 tcpu;
> +
> +       if (skb_rx_queue_recorded(skb)) {
> +               u16 index = skb_get_rx_queue(skb);
> +               if (unlikely(index >= dev->num_rx_queues)) {
> +                       WARN_ONCE(dev->num_rx_queues > 1, "%s received packet "
> +                               "on queue %u, but number of RX queues is %u\n",
> +                               dev->name, index, dev->num_rx_queues);
> +                       goto done;
> +               }
> +               rxqueue = dev->_rx + index;
> +       } else
> +               rxqueue = dev->_rx;
> +
> +       if (!rxqueue->rps_map && !rxqueue->rps_flow_table)
> +               goto done;
> +
> +       skb->rxhash = skb_get_rxhash(skb);

the return value of skb_get_rxhash() maybe skb->rxhash, so we don't
need to assign it to skb->rxhash again. Use a local variable to save
the value and __skb_get_rxhash() should cache the rxhash into
skb->rxhash for future use.

> +       if (skb->rxhash < 0)
> +               goto done;
> +
>        flow_table = rcu_dereference(rxqueue->rps_flow_table);
>        sock_flow_table = rcu_dereference(rps_sock_flow_table);
>        if (flow_table && sock_flow_table) {
>



-- 
Regards,
Changli Gao(xiaosuo@gmail.com)

^ permalink raw reply

* Re: [PATCH v4 2/2] macvtap: Implement multiqueue for macvtap driver
From: Changli Gao @ 2010-08-04 11:37 UTC (permalink / raw)
  To: Krishna Kumar; +Cc: davem, arnd, mst, netdev, bhutchings, therbert
In-Reply-To: <20100804105525.22212.45090.sendpatchset@krkumar2.in.ibm.com>

On Wed, Aug 4, 2010 at 6:55 PM, Krishna Kumar <krkumar2@in.ibm.com> wrote:
> From: Krishna Kumar <krkumar2@in.ibm.com>
>
> Implement multiqueue facility for macvtap driver. The idea is that
> a macvtap device can be opened multiple times and the fd's can be
> used to register eg, as backend for vhost.
>
> Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
> ---
>  drivers/net/macvtap.c      |   89 ++++++++++++++++++++++++++++-------
>  include/linux/if_macvlan.h |    9 +++
>  2 files changed, 80 insertions(+), 18 deletions(-)
>
> diff -ruNp org/drivers/net/macvtap.c new/drivers/net/macvtap.c
> --- org/drivers/net/macvtap.c   2010-07-28 15:10:10.000000000 +0530
> +++ new/drivers/net/macvtap.c   2010-08-04 16:19:32.000000000 +0530
> @@ -84,26 +84,45 @@ static const struct proto_ops macvtap_so
>  static DEFINE_SPINLOCK(macvtap_lock);
>
>  /*
> - * Choose the next free queue, for now there is only one
> + * get_slot: return a [unused/occupied] slot in vlan->taps[]:
> + *     - if 'q' is NULL, return the first empty slot;
> + *     - otherwise, return the slot this pointer occupies.
>  */
> +static int get_slot(struct macvlan_dev *vlan, struct macvtap_queue *q)
> +{
> +       int i;
> +
> +       for (i = 0; i < MAX_MACVTAP_QUEUES; i++) {
> +               if (rcu_dereference(vlan->taps[i]) == q)
> +                       return i;
> +       }
> +
> +       /* Should never happen */
> +       BUG_ON(1);
> +}
> +
>  static int macvtap_set_queue(struct net_device *dev, struct file *file,
>                                struct macvtap_queue *q)
>  {
>        struct macvlan_dev *vlan = netdev_priv(dev);
> +       int index;
>        int err = -EBUSY;
>
>        spin_lock(&macvtap_lock);
> -       if (rcu_dereference(vlan->tap))
> +       if (vlan->numvtaps == MAX_MACVTAP_QUEUES)
>                goto out;
>
>        err = 0;
> +       index = get_slot(vlan, NULL);
>        rcu_assign_pointer(q->vlan, vlan);
> -       rcu_assign_pointer(vlan->tap, q);
> +       rcu_assign_pointer(vlan->taps[index], q);
>        sock_hold(&q->sk);
>
>        q->file = file;
>        file->private_data = q;
>
> +       vlan->numvtaps++;
> +
>  out:
>        spin_unlock(&macvtap_lock);
>        return err;
> @@ -124,9 +143,12 @@ static void macvtap_put_queue(struct mac
>        spin_lock(&macvtap_lock);
>        vlan = rcu_dereference(q->vlan);
>        if (vlan) {
> -               rcu_assign_pointer(vlan->tap, NULL);
> +               int index = get_slot(vlan, q);
> +
> +               rcu_assign_pointer(vlan->taps[index], NULL);
>                rcu_assign_pointer(q->vlan, NULL);
>                sock_put(&q->sk);
> +               --vlan->numvtaps;
>        }
>
>        spin_unlock(&macvtap_lock);
> @@ -136,39 +158,72 @@ static void macvtap_put_queue(struct mac
>  }
>
>  /*
> - * Since we only support one queue, just dereference the pointer.
> + * Select a queue based on the rxq of the device on which this packet
> + * arrived. If the incoming device is not mq, calculate a flow hash to
> + * select a queue. vlan->numvtaps is cached in case it reduces during
> + * the execution of this function.
>  */
>  static struct macvtap_queue *macvtap_get_queue(struct net_device *dev,
>                                               struct sk_buff *skb)
>  {
>        struct macvlan_dev *vlan = netdev_priv(dev);
> +       struct macvtap_queue *tap = NULL;
> +       int numvtaps = vlan->numvtaps;
> +       __u32 rxq;
> +
> +       if (!numvtaps)
> +               goto out;
> +
> +       if (likely(skb_rx_queue_recorded(skb))) {
> +               rxq = skb_get_rx_queue(skb);
> +
> +               while (unlikely(rxq >= numvtaps))
> +                       rxq -= numvtaps;
>
> -       return rcu_dereference(vlan->tap);
> +               tap = rcu_dereference(vlan->taps[rxq]);
> +               if (tap)
> +                       goto out;
> +       }
> +
> +       rxq = skb_get_rxhash(skb);
> +       if (!rxq)
> +               rxq = smp_processor_id();
> +
> +       tap = rcu_dereference(vlan->taps[rxq & (numvtaps - 1)]);
> +

numvtaps maybe not power of 2. So some queue maybe can't be used.
You'd better maintain a online queue map and get the index as
get_rps_cpu() dones.

((u64)rxhash * numvtaps) >> 32

for the sbks, we can't get a valid rxhash, we'd better pass them to a
specified queue, such as queue 0.

> +out:
> +       return tap;
>  }
>
>  /*
>  * The net_device is going away, give up the reference
> - * that it holds on the queue (all the queues one day)
> - * and safely set the pointer from the queues to NULL.
> + * that it holds on all queues and safely set the pointer
> + * from the queues to NULL.
>  */
>  static void macvtap_del_queues(struct net_device *dev)
>  {
>        struct macvlan_dev *vlan = netdev_priv(dev);
> -       struct macvtap_queue *q;
> +       struct macvtap_queue *q, *qlist[MAX_MACVTAP_QUEUES];
> +       int i, j = 0;
>
> +       /* macvtap_put_queue can free some slots, so go through all slots */
>        spin_lock(&macvtap_lock);
> -       q = rcu_dereference(vlan->tap);
> -       if (!q) {
> -               spin_unlock(&macvtap_lock);
> -               return;
> +       for (i = 0; i < MAX_MACVTAP_QUEUES && vlan->numvtaps; i++) {
> +               q = rcu_dereference(vlan->taps[i]);
> +               if (q) {
> +                       qlist[j++] = q;
> +                       rcu_assign_pointer(vlan->taps[i], NULL);
> +                       rcu_assign_pointer(q->vlan, NULL);
> +                       vlan->numvtaps--;
> +               }
>        }
> -
> -       rcu_assign_pointer(vlan->tap, NULL);
> -       rcu_assign_pointer(q->vlan, NULL);
> +       BUG_ON(vlan->numvtaps != 0);
>        spin_unlock(&macvtap_lock);
>
>        synchronize_rcu();
> -       sock_put(&q->sk);
> +
> +       for (--j; j >= 0; j--)
> +               sock_put(&qlist[j]->sk);
>  }
>
>  /*
> diff -ruNp org/include/linux/if_macvlan.h new/include/linux/if_macvlan.h
> --- org/include/linux/if_macvlan.h      2010-07-28 15:10:10.000000000 +0530
> +++ new/include/linux/if_macvlan.h      2010-08-04 16:19:32.000000000 +0530
> @@ -40,6 +40,12 @@ struct macvlan_rx_stats {
>        unsigned long           rx_errors;
>  };
>
> +/*
> + * Maximum times a macvtap device can be opened. This can be used to
> + * configure the number of receive queue, e.g. for multiqueue virtio.
> + */
> +#define MAX_MACVTAP_QUEUES     (NR_CPUS < 16 ? NR_CPUS : 16)
> +
>  struct macvlan_dev {
>        struct net_device       *dev;
>        struct list_head        list;
> @@ -50,7 +56,8 @@ struct macvlan_dev {
>        enum macvlan_mode       mode;
>        int (*receive)(struct sk_buff *skb);
>        int (*forward)(struct net_device *dev, struct sk_buff *skb);
> -       struct macvtap_queue    *tap;
> +       struct macvtap_queue    *taps[MAX_MACVTAP_QUEUES];
> +       int                     numvtaps;
>  };
>
>  static inline void macvlan_count_rx(const struct macvlan_dev *vlan,
>



-- 
Regards,
Changli Gao(xiaosuo@gmail.com)

^ permalink raw reply

* Re: [PATCH v4 1/2] core: Factor out flow calculation from get_rps_cpu
From: Krishna Kumar2 @ 2010-08-04 12:02 UTC (permalink / raw)
  To: Changli Gao; +Cc: arnd, bhutchings, davem, mst, netdev, therbert
In-Reply-To: <AANLkTikfW_PfYVfA-9cSx8-j16OuH6mr9K32apPj6nLy@mail.gmail.com>

Changli Gao <xiaosuo@gmail.com> wrote on 08/04/2010 04:52:58 PM:

> > +#ifdef CONFIG_RPS
> > +
> > +/* One global table that all flow-based protocols share. */
> > +struct rps_sock_flow_table *rps_sock_flow_table __read_mostly;
> > +EXPORT_SYMBOL(rps_sock_flow_table);
> > +
> > +/*
> > + * get_rps_cpu is called from netif_receive_skb and returns the target
> > + * CPU from the RPS map of the receiving queue for a given skb.
> > + * rcu_read_lock must be held on entry.
> > + */
> > +static int get_rps_cpu(struct net_device *dev, struct sk_buff *skb,
> > +                      struct rps_dev_flow **rflowp)
> > +{
> > +       struct netdev_rx_queue *rxqueue;
> > +       struct rps_map *map;
> > +       struct rps_dev_flow_table *flow_table;
> > +       struct rps_sock_flow_table *sock_flow_table;
> > +       int cpu = -1;
> > +       u16 tcpu;
> > +
> > +       if (skb_rx_queue_recorded(skb)) {
> > +               u16 index = skb_get_rx_queue(skb);
> > +               if (unlikely(index >= dev->num_rx_queues)) {
> > +                       WARN_ONCE(dev->num_rx_queues > 1, "%s
> received packet "
> > +                               "on queue %u, but number of RX
> queues is %u\n",
> > +                               dev->name, index, dev->num_rx_queues);
> > +                       goto done;
> > +               }
> > +               rxqueue = dev->_rx + index;
> > +       } else
> > +               rxqueue = dev->_rx;
> > +
> > +       if (!rxqueue->rps_map && !rxqueue->rps_flow_table)
> > +               goto done;
> > +
> > +       skb->rxhash = skb_get_rxhash(skb);
>
> the return value of skb_get_rxhash() maybe skb->rxhash, so we don't
> need to assign it to skb->rxhash again. Use a local variable to save
> the value and __skb_get_rxhash() should cache the rxhash into
> skb->rxhash for future use.

I wanted the function to return a rxhash instead of save+return,
since other users, eg macvtap, doesn't need the rxhash beyond
calculating a rxq with that hash. So for those users, we can
avoid updating both skb->rxhash and a local variable (since with
your suggestion, rps will update either one or two variables
depending on whether rxhash is cached or not). I am not sure
which is better.

Thanks,

- KK


^ permalink raw reply

* [PATCH] RxRPC: Fix a potential deadlock between the call resend_timer and state_lock
From: David Howells @ 2010-08-04 12:34 UTC (permalink / raw)
  To: netdev; +Cc: David Howells

RxRPC can potentially deadlock as rxrpc_resend_time_expired() wants to get
call->state_lock so that it can alter the state of an RxRPC call.  However, its
caller (call_timer_fn()) has an apparent lock on the timer struct.

The problem is that rxrpc_resend_time_expired() isn't permitted to lock
call->state_lock as this could cause a deadlock against rxrpc_send_abort() as
that takes state_lock and then attempts to delete the resend timer by calling
del_timer_sync().

The deadlock can occur because del_timer_sync() will sit there forever waiting
for rxrpc_resend_time_expired() to return, but the latter may then wait for
call->state_lock, which rxrpc_send_abort() holds around del_timer_sync()...

This leads to a warning appearing in the kernel log that looks something like
the attached.

It should be sufficient to simply dispense with the locks.  It doesn't matter
if we set the resend timer expired event bit and queue the event processor
whilst we're changing state to one where the resend timer is irrelevant as the
event can just be ignored by the processor thereafter.


=======================================================
[ INFO: possible circular locking dependency detected ]
2.6.35-rc3-cachefs+ #115
-------------------------------------------------------
swapper/0 is trying to acquire lock:
 (&call->state_lock){++--..}, at: [<ffffffffa00200d4>] rxrpc_resend_time_expired+0x56/0x96 [af_rxrpc]

but task is already holding lock:
 (&call->resend_timer){+.-...}, at: [<ffffffff8103b675>] run_timer_softirq+0x182/0x2a5

which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

-> #1 (&call->resend_timer){+.-...}:
       [<ffffffff810560bc>] __lock_acquire+0x889/0x8fa
       [<ffffffff81056184>] lock_acquire+0x57/0x6d
       [<ffffffff8103bb9c>] del_timer_sync+0x3c/0x86
       [<ffffffffa002bb7a>] rxrpc_send_abort+0x50/0x97 [af_rxrpc]
       [<ffffffffa002bdd9>] rxrpc_kernel_abort_call+0xa1/0xdd [af_rxrpc]
       [<ffffffffa0061588>] afs_deliver_to_call+0x129/0x368 [kafs]
       [<ffffffffa006181b>] afs_process_async_call+0x54/0xff [kafs]
       [<ffffffff8104261d>] worker_thread+0x1ef/0x2e2
       [<ffffffff81045f47>] kthread+0x7a/0x82
       [<ffffffff81002cd4>] kernel_thread_helper+0x4/0x10

-> #0 (&call->state_lock){++--..}:
       [<ffffffff81055237>] validate_chain+0x727/0xd23
       [<ffffffff810560bc>] __lock_acquire+0x889/0x8fa
       [<ffffffff81056184>] lock_acquire+0x57/0x6d
       [<ffffffff813e6b69>] _raw_read_lock_bh+0x34/0x43
       [<ffffffffa00200d4>] rxrpc_resend_time_expired+0x56/0x96 [af_rxrpc]
       [<ffffffff8103b6e6>] run_timer_softirq+0x1f3/0x2a5
       [<ffffffff81036828>] __do_softirq+0xa2/0x13e
       [<ffffffff81002dcc>] call_softirq+0x1c/0x28
       [<ffffffff810049f0>] do_softirq+0x38/0x80
       [<ffffffff810361a2>] irq_exit+0x45/0x47
       [<ffffffff81018fb3>] smp_apic_timer_interrupt+0x88/0x96
       [<ffffffff81002893>] apic_timer_interrupt+0x13/0x20
       [<ffffffff810011ac>] cpu_idle+0x4d/0x83
       [<ffffffff813e06f3>] start_secondary+0x1bd/0x1c1

other info that might help us debug this:

1 lock held by swapper/0:
 #0:  (&call->resend_timer){+.-...}, at: [<ffffffff8103b675>] run_timer_softirq+0x182/0x2a5

stack backtrace:
Pid: 0, comm: swapper Not tainted 2.6.35-rc3-cachefs+ #115
Call Trace:
 <IRQ>  [<ffffffff81054414>] print_circular_bug+0xae/0xbd
 [<ffffffff81055237>] validate_chain+0x727/0xd23
 [<ffffffff810560bc>] __lock_acquire+0x889/0x8fa
 [<ffffffff810539a7>] ? mark_lock+0x42f/0x51f
 [<ffffffff81056184>] lock_acquire+0x57/0x6d
 [<ffffffffa00200d4>] ? rxrpc_resend_time_expired+0x56/0x96 [af_rxrpc]
 [<ffffffff813e6b69>] _raw_read_lock_bh+0x34/0x43
 [<ffffffffa00200d4>] ? rxrpc_resend_time_expired+0x56/0x96 [af_rxrpc]
 [<ffffffffa00200d4>] rxrpc_resend_time_expired+0x56/0x96 [af_rxrpc]
 [<ffffffff8103b6e6>] run_timer_softirq+0x1f3/0x2a5
 [<ffffffff8103b675>] ? run_timer_softirq+0x182/0x2a5
 [<ffffffffa002007e>] ? rxrpc_resend_time_expired+0x0/0x96 [af_rxrpc]
 [<ffffffff810367ef>] ? __do_softirq+0x69/0x13e
 [<ffffffff81036828>] __do_softirq+0xa2/0x13e
 [<ffffffff81002dcc>] call_softirq+0x1c/0x28
 [<ffffffff810049f0>] do_softirq+0x38/0x80
 [<ffffffff810361a2>] irq_exit+0x45/0x47
 [<ffffffff81018fb3>] smp_apic_timer_interrupt+0x88/0x96
 [<ffffffff81002893>] apic_timer_interrupt+0x13/0x20
 <EOI>  [<ffffffff81049de1>] ? __atomic_notifier_call_chain+0x0/0x86
 [<ffffffff8100955b>] ? mwait_idle+0x6e/0x78
 [<ffffffff81009552>] ? mwait_idle+0x65/0x78
 [<ffffffff810011ac>] cpu_idle+0x4d/0x83
 [<ffffffff813e06f3>] start_secondary+0x1bd/0x1c1

Signed-off-by: David Howells <dhowells@redhat.com>
---

 net/rxrpc/ar-ack.c  |    3 +++
 net/rxrpc/ar-call.c |    6 ++----
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/net/rxrpc/ar-ack.c b/net/rxrpc/ar-ack.c
index 2714da1..b6ffe4e 100644
--- a/net/rxrpc/ar-ack.c
+++ b/net/rxrpc/ar-ack.c
@@ -245,6 +245,9 @@ static void rxrpc_resend_timer(struct rxrpc_call *call)
 	_enter("%d,%d,%d",
 	       call->acks_tail, call->acks_unacked, call->acks_head);
 
+	if (call->state >= RXRPC_CALL_COMPLETE)
+		return;
+
 	resend = 0;
 	resend_at = 0;
 
diff --git a/net/rxrpc/ar-call.c b/net/rxrpc/ar-call.c
index 909d092..bf656c2 100644
--- a/net/rxrpc/ar-call.c
+++ b/net/rxrpc/ar-call.c
@@ -786,6 +786,7 @@ static void rxrpc_call_life_expired(unsigned long _call)
 
 /*
  * handle resend timer expiry
+ * - may not take call->state_lock as this can deadlock against del_timer_sync()
  */
 static void rxrpc_resend_time_expired(unsigned long _call)
 {
@@ -796,12 +797,9 @@ static void rxrpc_resend_time_expired(unsigned long _call)
 	if (call->state >= RXRPC_CALL_COMPLETE)
 		return;
 
-	read_lock_bh(&call->state_lock);
 	clear_bit(RXRPC_CALL_RUN_RTIMER, &call->flags);
-	if (call->state < RXRPC_CALL_COMPLETE &&
-	    !test_and_set_bit(RXRPC_CALL_RESEND_TIMER, &call->events))
+	if (!test_and_set_bit(RXRPC_CALL_RESEND_TIMER, &call->events))
 		rxrpc_queue_call(call);
-	read_unlock_bh(&call->state_lock);
 }
 
 /*


^ permalink raw reply related

* Re: [PATCH v4 2/2] macvtap: Implement multiqueue for macvtap driver
From: Krishna Kumar2 @ 2010-08-04 13:25 UTC (permalink / raw)
  To: Changli Gao; +Cc: arnd, bhutchings, davem, mst, netdev, therbert
In-Reply-To: <AANLkTimgSg+iFouARaFv8X6S8d2e6L+ffkZkoOaja20R@mail.gmail.com>

Changli Gao <xiaosuo@gmail.com> wrote on 08/04/2010 05:07:23 PM:

Thanks for your review and comments!

> >  static struct macvtap_queue *macvtap_get_queue(struct net_device *dev,
> >                                               struct sk_buff *skb)
> >  {
> >        struct macvlan_dev *vlan = netdev_priv(dev);
> > +       struct macvtap_queue *tap = NULL;
> > +       int numvtaps = vlan->numvtaps;
> > +       __u32 rxq;
> > +
> > +       if (!numvtaps)
> > +               goto out;
> > +
> > +       if (likely(skb_rx_queue_recorded(skb))) {
> > +               rxq = skb_get_rx_queue(skb);
> > +
> > +               while (unlikely(rxq >= numvtaps))
> > +                       rxq -= numvtaps;
> >
> > -       return rcu_dereference(vlan->tap);
> > +               tap = rcu_dereference(vlan->taps[rxq]);
> > +               if (tap)
> > +                       goto out;
> > +       }
> > +
> > +       rxq = skb_get_rxhash(skb);
> > +       if (!rxq)
> > +               rxq = smp_processor_id();
> > +
> > +       tap = rcu_dereference(vlan->taps[rxq & (numvtaps - 1)]);
> > +
>
> numvtaps maybe not power of 2. So some queue maybe can't be used.
> You'd better maintain a online queue map and get the index as
> get_rps_cpu() dones.
>
> ((u64)rxhash * numvtaps) >> 32
>
> for the sbks, we can't get a valid rxhash, we'd better pass them to a
> specified queue, such as queue 0.

macvtap *with* mq support would be used with mq devices - you
open multiple queues on the macvtap device depending on the
number of queues for the physical device. So since this is an
unlikely case (as can be seen in the patch, and I guess I
should add another "likely" to the "if (tap)" check since fd's
should not be closed), I guess a simple % can be used. Does
the following sound reasonable?

1. Use % to find the slot.
2. If slot is null - I don't want to handle this since I think
   it is better to return NULL if some fd's were closed by user.
   Typically this should never happen since fd's are opened and
   passed to vhost for setting up the backend. So if they are
   closed, then I think NULL is OK.

Arnd, please let me know what you would also suggest.

- KK


^ permalink raw reply

* Re: NET_NS: unregister_netdevice: waiting for lo to become free (after using openvpn) (was Re: sysfs bug when using tun with network namespaces)
From: Michael Leun @ 2010-08-04 13:35 UTC (permalink / raw)
  To: greg; +Cc: netdev, davem, linux-kernel
In-Reply-To: <20100711192939.1c25dcaf@xenia.leun.net>

Hi,

On Sun, 11 Jul 2010 19:29:39 +0200
Michael Leun <lkml20100708@newton.leun.net> wrote:

[...]
> Jul 10 20:02:36 doris kernel: unregister_netdevice: waiting for lo to
> become free. Usage count = 3 [repeated]
> 
> Now one might say it is fault of openvpn (used OpenVPN 2.1_rc20
> i586-suse-linux - the one in openSuSE 11.2 package), openvpn didn't
> close some ressource and ssh does fine.
> 
> But: should'nt kernel clean up after process when it exits?
> And/or: Should'nt kernel clean up if last process in network namespace
> exits - there is nothing left which might use that interface?!
> 
> Greg KH <greg@kroah.com> wrote:
> 
> > Yes, you are correct.  Care to resend all of this to the
> > network-namespace developer(s) and the netdev mailing list so that
> > the correct people are notified so they can fix it all?
> 
> [X] done - hopefully, cannot find a particular network namespace
> developer in MAINTAINERS or source files. If such a one exists, please
> forward.

Did'nt work. Got no reaction from network mailinglist at all and bug
still is in 2.6.35.

-- 
MfG,

Michael Leun

^ permalink raw reply

* Re: [PATCH v4 1/2] core: Factor out flow calculation from get_rps_cpu
From: Changli Gao @ 2010-08-04 13:38 UTC (permalink / raw)
  To: Krishna Kumar2; +Cc: arnd, bhutchings, davem, mst, netdev, therbert
In-Reply-To: <OFE9752776.5432C83A-ON65257775.003F1B51-65257775.00420641@in.ibm.com>

On Wed, Aug 4, 2010 at 8:02 PM, Krishna Kumar2 <krkumar2@in.ibm.com> wrote:
> Changli Gao <xiaosuo@gmail.com> wrote on 08/04/2010 04:52:58 PM:
>>
>> the return value of skb_get_rxhash() maybe skb->rxhash, so we don't
>> need to assign it to skb->rxhash again. Use a local variable to save
>> the value and __skb_get_rxhash() should cache the rxhash into
>> skb->rxhash for future use.
>
> I wanted the function to return a rxhash instead of save+return,
> since other users, eg macvtap, doesn't need the rxhash beyond
> calculating a rxq with that hash. So for those users, we can
> avoid updating both skb->rxhash and a local variable (since with
> your suggestion, rps will update either one or two variables
> depending on whether rxhash is cached or not). I am not sure
> which is better.
>

rxhash can also be generated by NIC(hardware) and used some where
else. So for these users, generating the rxhash again is wasting time.
Thanks.


-- 
Regards,
Changli Gao(xiaosuo@gmail.com)

^ permalink raw reply

* Re: [PATCH] cls_flow: add sanity check for the packet length
From: jamal @ 2010-08-04 13:39 UTC (permalink / raw)
  To: Changli Gao; +Cc: Patrick McHardy, David S. Miller, netdev
In-Reply-To: <1280905687-8463-1-git-send-email-xiaosuo@gmail.com>

On Wed, 2010-08-04 at 15:08 +0800, Changli Gao wrote: 
> The packet length should be checked before the packet data is dereferenced.
> 
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> ---
>  include/linux/skbuff.h |    6 ++--
>  net/sched/cls_flow.c   |   69 +++++++++++++++++++++++++++++++++----------------
>  2 files changed, 50 insertions(+), 25 deletions(-)
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index d20d9e7..3f0ac50 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -1210,12 +1210,12 @@ static inline unsigned char *pskb_pull(struct sk_buff *skb, unsigned int len)
>  	return unlikely(len > skb->len) ? NULL : __pskb_pull(skb, len);
>  }
>  
> -static inline int pskb_may_pull(struct sk_buff *skb, unsigned int len)
> +static inline bool pskb_may_pull(struct sk_buff *skb, unsigned int len)
>  {
>  	if (likely(len <= skb_headlen(skb)))
> -		return 1;
> +		return true;
>  	if (unlikely(len > skb->len))
> -		return 0;
> +		return false;
>  	return __pskb_pull_tail(skb, len - skb_headlen(skb)) != NULL;
>  }

Above is a different patch?

> diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c
> index f73542d..d63a374 100644
> --- a/net/sched/cls_flow.c
> +++ b/net/sched/cls_flow.c
> @@ -65,37 +65,48 @@ static inline u32 addr_fold(void *addr)
>  	return (a & 0xFFFFFFFF) ^ (BITS_PER_LONG > 32 ? a >> 32 : 0);
>  }
>  
> -static u32 flow_get_src(const struct sk_buff *skb)
> +static inline bool flow_pskb_may_pull(struct sk_buff *skb, unsigned int len)
> +{
> +	return pskb_may_pull(skb, skb_network_offset(skb) + len);
> +}

network_pskb_may_pull() would make it more generic (probably in
skbuff.h) and reused everywhere you need skb_network_offset


> +static u32 flow_get_src(struct sk_buff *skb)
>  {
>  	switch (skb->protocol) {
>  	case htons(ETH_P_IP):
> -		return ntohl(ip_hdr(skb)->saddr);
> +		return flow_pskb_may_pull(skb, sizeof(struct iphdr)) ?
> +		       ntohl(ip_hdr(skb)->saddr) : 0;
>  	case htons(ETH_P_IPV6):
> -		return ntohl(ipv6_hdr(skb)->saddr.s6_addr32[3]);
> +		return flow_pskb_may_pull(skb, sizeof(struct ipv6hdr)) ?
> +		       ntohl(ipv6_hdr(skb)->saddr.s6_addr32[3]) : 0;
>  	default:


BTW - does returning zero above make sense or is returning the default below better?

>  		return addr_fold(skb->sk);

Same comment applies in all other switch statements..

cheers,
jamal


^ permalink raw reply

* Re: [PATCH v4 2/2] macvtap: Implement multiqueue for macvtap driver
From: Changli Gao @ 2010-08-04 13:42 UTC (permalink / raw)
  To: Krishna Kumar2; +Cc: arnd, bhutchings, davem, mst, netdev, therbert
In-Reply-To: <OF3D82198C.4738EB40-ON65257775.0046DC1D-65257775.004997E7@in.ibm.com>

On Wed, Aug 4, 2010 at 9:25 PM, Krishna Kumar2 <krkumar2@in.ibm.com> wrote:
> Changli Gao <xiaosuo@gmail.com> wrote on 08/04/2010 05:07:23 PM:
>
> Thanks for your review and comments!
>
>> >  static struct macvtap_queue *macvtap_get_queue(struct net_device *dev,
>> >                                               struct sk_buff *skb)
>> >  {
>> >        struct macvlan_dev *vlan = netdev_priv(dev);
>> > +       struct macvtap_queue *tap = NULL;
>> > +       int numvtaps = vlan->numvtaps;
>> > +       __u32 rxq;
>> > +
>> > +       if (!numvtaps)
>> > +               goto out;
>> > +
>> > +       if (likely(skb_rx_queue_recorded(skb))) {
>> > +               rxq = skb_get_rx_queue(skb);
>> > +
>> > +               while (unlikely(rxq >= numvtaps))
>> > +                       rxq -= numvtaps;
>> >
>> > -       return rcu_dereference(vlan->tap);
>> > +               tap = rcu_dereference(vlan->taps[rxq]);
>> > +               if (tap)
>> > +                       goto out;
>> > +       }
>> > +
>> > +       rxq = skb_get_rxhash(skb);
>> > +       if (!rxq)
>> > +               rxq = smp_processor_id();
>> > +
>> > +       tap = rcu_dereference(vlan->taps[rxq & (numvtaps - 1)]);
>> > +
>>
>> numvtaps maybe not power of 2. So some queue maybe can't be used.
>> You'd better maintain a online queue map and get the index as
>> get_rps_cpu() dones.
>>
>> ((u64)rxhash * numvtaps) >> 32
>>
>> for the sbks, we can't get a valid rxhash, we'd better pass them to a
>> specified queue, such as queue 0.
>
> macvtap *with* mq support would be used with mq devices - you
> open multiple queues on the macvtap device depending on the
> number of queues for the physical device. So since this is an
> unlikely case (as can be seen in the patch, and I guess I
> should add another "likely" to the "if (tap)" check since fd's
> should not be closed), I guess a simple % can be used. Does
> the following sound reasonable?
>
> 1. Use % to find the slot.

It is slower than the method used by get_rps_cpu().

> 2. If slot is null - I don't want to handle this since I think
>   it is better to return NULL if some fd's were closed by user.
>   Typically this should never happen since fd's are opened and
>   passed to vhost for setting up the backend. So if they are
>   closed, then I think NULL is OK.
>
> Arnd, please let me know what you would also suggest.
>
> - KK
>
>



-- 
Regards,
Changli Gao(xiaosuo@gmail.com)

^ permalink raw reply

* Re: [PATCH] cls_rsvp: add sanity check for the packet length
From: jamal @ 2010-08-04 13:42 UTC (permalink / raw)
  To: Changli Gao; +Cc: David S. Miller, netdev
In-Reply-To: <1280912124-30374-1-git-send-email-xiaosuo@gmail.com>

On Wed, 2010-08-04 at 16:55 +0800, Changli Gao wrote:
> The packet length should be checked before the packet data is dereferenced.
> 
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> ---
>  net/sched/cls_rsvp.h |   10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> diff --git a/net/sched/cls_rsvp.h b/net/sched/cls_rsvp.h
> index dd9414e..4fa119d 100644
> --- a/net/sched/cls_rsvp.h
> +++ b/net/sched/cls_rsvp.h
> @@ -143,9 +143,17 @@ static int rsvp_classify(struct sk_buff *skb, struct tcf_proto *tp,
>  	u8 tunnelid = 0;
>  	u8 *xprt;
>  #if RSVP_DST_LEN == 4
> -	struct ipv6hdr *nhptr = ipv6_hdr(skb);
> +	struct ipv6hdr *nhptr;
> +
> +	if (!pskb_may_pull(skb, skb_network_offset(skb) + sizeof(*nhptr)))
> +		return -1;
> +	nhptr = ipv6_hdr(skb);
>  #else
>  	struct iphdr *nhptr = ip_hdr(skb);
> +
> +	if (!pskb_may_pull(skb, skb_network_offset(skb) + sizeof(*nhptr)))
> +		return -1;
> +	nhptr = ip_hdr(skb);
>  #endif

Maybe a good time to move nhptr declaration outside #if since it is used
in #else as well.

Otherwise:
Acked-by: Jamal Hadi Salim <hadi@cyberus.ca>

cheers,
jamal


^ permalink raw reply

* Re: [PATCH] cls_rsvp: add sanity check for the packet length
From: Changli Gao @ 2010-08-04 13:44 UTC (permalink / raw)
  To: hadi; +Cc: David S. Miller, netdev
In-Reply-To: <1280929327.5669.10.camel@bigi>

On Wed, Aug 4, 2010 at 9:42 PM, jamal <hadi@cyberus.ca> wrote:
> On Wed, 2010-08-04 at 16:55 +0800, Changli Gao wrote:
>> The packet length should be checked before the packet data is dereferenced.
>>
>> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
>> ---
>>  net/sched/cls_rsvp.h |   10 +++++++++-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>> diff --git a/net/sched/cls_rsvp.h b/net/sched/cls_rsvp.h
>> index dd9414e..4fa119d 100644
>> --- a/net/sched/cls_rsvp.h
>> +++ b/net/sched/cls_rsvp.h
>> @@ -143,9 +143,17 @@ static int rsvp_classify(struct sk_buff *skb, struct tcf_proto *tp,
>>       u8 tunnelid = 0;
>>       u8 *xprt;
>>  #if RSVP_DST_LEN == 4
>> -     struct ipv6hdr *nhptr = ipv6_hdr(skb);
>> +     struct ipv6hdr *nhptr;
>> +
>> +     if (!pskb_may_pull(skb, skb_network_offset(skb) + sizeof(*nhptr)))
>> +             return -1;
>> +     nhptr = ipv6_hdr(skb);
>>  #else
>>       struct iphdr *nhptr = ip_hdr(skb);
>> +
>> +     if (!pskb_may_pull(skb, skb_network_offset(skb) + sizeof(*nhptr)))
>> +             return -1;
>> +     nhptr = ip_hdr(skb);
>>  #endif
>
> Maybe a good time to move nhptr declaration outside #if since it is used
> in #else as well.
>

They are not the same type. I am afraid that it won't work.

-- 
Regards,
Changli Gao(xiaosuo@gmail.com)

^ 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