* [PATCH 2/2] net/macvtap: add vhost support
From: Arnd Bergmann @ 2010-02-13 10:35 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, Ed Swierk, Patrick McHardy, Sridhar Samudrala,
qemu-devel, Michael S. Tsirkin
In-Reply-To: <201002131133.43028.arnd@arndb.de>
This adds support for passing a macvtap file descriptor into
vhost-net, much like we already do for tun/tap.
Most of the new code is taken from the respective patch
in the tun driver and may get consolidated in the future.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/macvtap.c | 98 ++++++++++++++++++++++++++++++++++---------
drivers/vhost/net.c | 8 +++-
include/linux/if_macvlan.h | 13 ++++++
3 files changed, 96 insertions(+), 23 deletions(-)
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 7050997..e354501 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -58,6 +58,8 @@ static unsigned int macvtap_major;
static struct class *macvtap_class;
static struct cdev macvtap_cdev;
+static const struct proto_ops macvtap_socket_ops;
+
/*
* RCU usage:
* The macvtap_queue and the macvlan_dev are loosely coupled, the
@@ -176,7 +178,7 @@ static int macvtap_forward(struct net_device *dev, struct sk_buff *skb)
return -ENOLINK;
skb_queue_tail(&q->sk.sk_receive_queue, skb);
- wake_up(q->sk.sk_sleep);
+ wake_up_interruptible_poll(q->sk.sk_sleep, POLLIN | POLLRDNORM | POLLRDBAND);
return 0;
}
@@ -242,7 +244,7 @@ static void macvtap_sock_write_space(struct sock *sk)
return;
if (sk->sk_sleep && waitqueue_active(sk->sk_sleep))
- wake_up_interruptible_sync(sk->sk_sleep);
+ wake_up_interruptible_poll(sk->sk_sleep, POLLOUT | POLLWRNORM | POLLWRBAND);
}
static int macvtap_open(struct inode *inode, struct file *file)
@@ -270,6 +272,8 @@ static int macvtap_open(struct inode *inode, struct file *file)
init_waitqueue_head(&q->sock.wait);
q->sock.type = SOCK_RAW;
q->sock.state = SS_CONNECTED;
+ q->sock.file = file;
+ q->sock.ops = &macvtap_socket_ops;
sock_init_data(&q->sock, &q->sk);
q->sk.sk_write_space = macvtap_sock_write_space;
@@ -387,32 +391,20 @@ static ssize_t macvtap_put_user(struct macvtap_queue *q,
rcu_read_lock_bh();
vlan = rcu_dereference(q->vlan);
- macvlan_count_rx(vlan, len, ret == 0, 0);
+ if (vlan)
+ macvlan_count_rx(vlan, len, ret == 0, 0);
rcu_read_unlock_bh();
return ret ? ret : len;
}
-static ssize_t macvtap_aio_read(struct kiocb *iocb, const struct iovec *iv,
- unsigned long count, loff_t pos)
+static ssize_t macvtap_do_read(struct macvtap_queue *q, struct kiocb *iocb,
+ const struct iovec *iv, unsigned long len,
+ int noblock)
{
- struct file *file = iocb->ki_filp;
- struct macvtap_queue *q = file->private_data;
-
DECLARE_WAITQUEUE(wait, current);
struct sk_buff *skb;
- ssize_t len, ret = 0;
-
- if (!q) {
- ret = -ENOLINK;
- goto out;
- }
-
- len = iov_length(iv, count);
- if (len < 0) {
- ret = -EINVAL;
- goto out;
- }
+ ssize_t ret = 0;
add_wait_queue(q->sk.sk_sleep, &wait);
while (len) {
@@ -421,7 +413,7 @@ static ssize_t macvtap_aio_read(struct kiocb *iocb, const struct iovec *iv,
/* Read frames from the queue */
skb = skb_dequeue(&q->sk.sk_receive_queue);
if (!skb) {
- if (file->f_flags & O_NONBLOCK) {
+ if (noblock) {
ret = -EAGAIN;
break;
}
@@ -440,7 +432,24 @@ static ssize_t macvtap_aio_read(struct kiocb *iocb, const struct iovec *iv,
current->state = TASK_RUNNING;
remove_wait_queue(q->sk.sk_sleep, &wait);
+ return ret;
+}
+
+static ssize_t macvtap_aio_read(struct kiocb *iocb, const struct iovec *iv,
+ unsigned long count, loff_t pos)
+{
+ struct file *file = iocb->ki_filp;
+ struct macvtap_queue *q = file->private_data;
+ ssize_t len, ret = 0;
+ len = iov_length(iv, count);
+ if (len < 0) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ ret = macvtap_do_read(q, iocb, iv, len, file->f_flags & O_NONBLOCK);
+ ret = min_t(ssize_t, ret, len); /* XXX copied from tun.c. Why? */
out:
return ret;
}
@@ -538,6 +547,53 @@ static const struct file_operations macvtap_fops = {
#endif
};
+static int macvtap_sendmsg(struct kiocb *iocb, struct socket *sock,
+ struct msghdr *m, size_t total_len)
+{
+ struct macvtap_queue *q = container_of(sock, struct macvtap_queue, sock);
+ return macvtap_get_user(q, m->msg_iov, total_len,
+ m->msg_flags & MSG_DONTWAIT);
+}
+
+static int macvtap_recvmsg(struct kiocb *iocb, struct socket *sock,
+ struct msghdr *m, size_t total_len,
+ int flags)
+{
+ struct macvtap_queue *q = container_of(sock, struct macvtap_queue, sock);
+ int ret;
+ if (flags & ~(MSG_DONTWAIT|MSG_TRUNC))
+ return -EINVAL;
+ ret = macvtap_do_read(q, iocb, m->msg_iov, total_len,
+ flags & MSG_DONTWAIT);
+ if (ret > total_len) {
+ m->msg_flags |= MSG_TRUNC;
+ ret = flags & MSG_TRUNC ? ret : total_len;
+ }
+ return ret;
+}
+
+/* Ops structure to mimic raw sockets with tun */
+static const struct proto_ops macvtap_socket_ops = {
+ .sendmsg = macvtap_sendmsg,
+ .recvmsg = macvtap_recvmsg,
+};
+
+/* Get an underlying socket object from tun file. Returns error unless file is
+ * attached to a device. The returned object works like a packet socket, it
+ * can be used for sock_sendmsg/sock_recvmsg. The caller is responsible for
+ * holding a reference to the file for as long as the socket is in use. */
+struct socket *macvtap_get_socket(struct file *file)
+{
+ struct macvtap_queue *q;
+ if (file->f_op != &macvtap_fops)
+ return ERR_PTR(-EINVAL);
+ q = file->private_data;
+ if (!q)
+ return ERR_PTR(-EBADFD);
+ return &q->sock;
+}
+EXPORT_SYMBOL_GPL(macvtap_get_socket);
+
static int macvtap_init(void)
{
int err;
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 4c89283..91a324c 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -22,6 +22,7 @@
#include <linux/if_packet.h>
#include <linux/if_arp.h>
#include <linux/if_tun.h>
+#include <linux/if_macvlan.h>
#include <net/sock.h>
@@ -452,13 +453,16 @@ err:
return ERR_PTR(r);
}
-static struct socket *get_tun_socket(int fd)
+static struct socket *get_tap_socket(int fd)
{
struct file *file = fget(fd);
struct socket *sock;
if (!file)
return ERR_PTR(-EBADF);
sock = tun_get_socket(file);
+ if (!IS_ERR(sock))
+ return sock;
+ sock = macvtap_get_socket(file);
if (IS_ERR(sock))
fput(file);
return sock;
@@ -473,7 +477,7 @@ static struct socket *get_socket(int fd)
sock = get_raw_socket(fd);
if (!IS_ERR(sock))
return sock;
- sock = get_tun_socket(fd);
+ sock = get_tap_socket(fd);
if (!IS_ERR(sock))
return sock;
return ERR_PTR(-ENOTSOCK);
diff --git a/include/linux/if_macvlan.h b/include/linux/if_macvlan.h
index 51f1512..7d7f1e3 100644
--- a/include/linux/if_macvlan.h
+++ b/include/linux/if_macvlan.h
@@ -7,6 +7,19 @@
#include <linux/netlink.h>
#include <net/netlink.h>
+#if defined(CONFIG_MACVTAP) || defined(CONFIG_MACVTAP_MODULE)
+struct socket *macvtap_get_socket(struct file *);
+#else
+#include <linux/err.h>
+#include <linux/errno.h>
+struct file;
+struct socket;
+static inline struct socket *macvtap_get_socket(struct file *f)
+{
+ return ERR_PTR(-EINVAL);
+}
+#endif /* CONFIG_MACVTAP */
+
struct macvlan_port;
struct macvtap_queue;
--
1.6.3.3
^ permalink raw reply related
* [PATCH] iputils: ping by mark
From: jamal @ 2010-02-13 15:25 UTC (permalink / raw)
To: YOSHIFUJI Hideaki; +Cc: netdev
[-- Attachment #1: Type: text/plain, Size: 154 bytes --]
I am CCing this to netdev. Can someone help me poke on
Yoshifuji-san? I cant reach him using electrons or
complex physical molecules...
cheers,
jamal
[-- Attachment #2: ping-mark --]
[-- Type: text/plain, Size: 2747 bytes --]
commit 7afb1e52ecc8bda3677f8b7db8433486936d473f
Author: Jamal Hadi Salim <hadi@cyberus.ca>
Date: Mon Oct 12 16:59:27 2009 -0400
[PATCH] iputils: ping by mark
This extends ping to send a packet out based on a given
mark using -m option. Useful with policy routing to take different paths
to same destination ..
Signed-off-by: Jamal Hadi Salim <hadi@cyberus.ca>
diff --git a/ping.c b/ping.c
index b67cff4..5c913e0 100644
--- a/ping.c
+++ b/ping.c
@@ -1216,7 +1216,7 @@ void usage(void)
fprintf(stderr,
"Usage: ping [-LRUbdfnqrvVaA] [-c count] [-i interval] [-w deadline]\n"
" [-p pattern] [-s packetsize] [-t ttl] [-I interface or address]\n"
-" [-M mtu discovery hint] [-S sndbuf]\n"
+" [-M mtu discovery hint] [-m mark] [-S sndbuf]\n"
" [ -T timestamp option ] [ -Q tos ] [hop1 ...] destination\n");
exit(2);
}
diff --git a/ping_common.c b/ping_common.c
index be36cbd..b1cc9fc 100644
--- a/ping_common.c
+++ b/ping_common.c
@@ -4,6 +4,7 @@
int options;
+int mark;
int sndbuf;
int ttl;
int rtt;
@@ -141,6 +142,17 @@ void common_options(int ch)
options |= F_INTERVAL;
break;
}
+ case 'm':
+ {
+ char *endp;
+ mark = (int)strtoul(optarg, &endp, 10);
+ if (mark < 0 || *endp != '\0') {
+ fprintf(stderr, "mark cannot be negative");
+ exit(2);
+ }
+ options |= F_MARK;
+ break;
+ }
case 'w':
deadline = atoi(optarg);
if (deadline < 0) {
@@ -442,6 +454,15 @@ void setup(int icmp_sock)
fprintf(stderr, "Warning: no SO_TIMESTAMP support, falling back to SIOCGSTAMP\n");
}
#endif
+ if (options & F_MARK) {
+ if (setsockopt(icmp_sock, SOL_SOCKET, SO_MARK,
+ &mark, sizeof(mark)) == -1) {
+ /* we probably dont wanna exit since old kernels
+ * dont support mark ..
+ */
+ fprintf(stderr, "Warning: Failed to set mark %d\n", mark);
+ }
+ }
/* Set some SNDTIMEO to prevent blocking forever
* on sends, when device is too slow or stalls. Just put limit
diff --git a/ping_common.h b/ping_common.h
index 5b80118..466792e 100644
--- a/ping_common.h
+++ b/ping_common.h
@@ -60,6 +60,7 @@ extern int options;
#define F_STRICTSOURCE 0x8000
#define F_NOLOOP 0x10000
#define F_TTL 0x20000
+#define F_MARK 0x40000
/*
* MAX_DUP_CHK is the number of bits in received table, i.e. the maximum
@@ -118,9 +119,9 @@ case 'a': case 'U': case 'c': case 'd': \
case 'f': case 'i': case 'w': case 'l': \
case 'S': case 'n': case 'p': case 'q': \
case 'r': case 's': case 'v': case 'L': \
-case 't': case 'A': case 'W': case 'B':
+case 't': case 'A': case 'W': case 'B': case 'm':
-#define COMMON_OPTSTR "h?VQ:I:M:aUc:dfi:w:l:S:np:qrs:vLt:AW:B"
+#define COMMON_OPTSTR "h?VQ:I:M:aUc:dfi:w:l:S:np:qrs:vLt:AW:Bm:"
/*
^ permalink raw reply related
* Re: [PATCH] iputils: ping by mark
From: YOSHIFUJI Hideaki @ 2010-02-13 15:38 UTC (permalink / raw)
To: hadi, YOSHIFUJI Hideaki, Linux Network Developers
In-Reply-To: <1266074714.6776.28.camel@bigi>
I notice this mail. I'll investigate the patch.
Thanks.
--yoshfuji
jamal wrote:
> I am CCing this to netdev. Can someone help me poke on
> Yoshifuji-san? I cant reach him using electrons or
> complex physical molecules...
>
> cheers,
> jamal
>
>
>
^ permalink raw reply
* Re: [PATCH] iputils: ping by mark
From: YOSHIFUJI Hideaki @ 2010-02-13 15:47 UTC (permalink / raw)
To: hadi; +Cc: netdev, YOSHIFUJI Hideaki
In-Reply-To: <1266074714.6776.28.camel@bigi>
Hello.
Okay, applied. Thank you.
Could you give me patch for doc/ping.sgml as well, please?
Regards,
--yoshfuji
jamal wrote:
> I am CCing this to netdev. Can someone help me poke on
> Yoshifuji-san? I cant reach him using electrons or
> complex physical molecules...
>
> cheers,
> jamal
>
>
>
^ permalink raw reply
* Re: [net-next PATCH v3 3/3] net: TCP thin dupack
From: Andreas Petlund @ 2010-02-13 15:50 UTC (permalink / raw)
To: Eric W. Biederman
Cc: netdev@vger.kernel.org, Ilpo Järvinen, Eric Dumazet,
Arnd Hannemann, LKML, shemminger, David Miller,
william.allen.simpson, damian
In-Reply-To: <m16361ubly.fsf@fess.ebiederm.org>
On 13. feb. 2010 03:13, Eric W. Biederman wrote:
> Andreas Petlund <apetlund@simula.no> writes:
>
>> Major changes:
>> -Possible to disable mechanisms by socket option
>> -Socket option value boundary check
>>
>>
>> Signed-off-by: Andreas Petlund <apetlund@simula.no>
>> ---
>> include/linux/sysctl.h | 1 +
>> include/linux/tcp.h | 4 +++-
>> include/net/tcp.h | 1 +
>> net/ipv4/sysctl_net_ipv4.c | 7 +++++++
>> net/ipv4/tcp.c | 7 +++++++
>> net/ipv4/tcp_input.c | 11 +++++++++++
>> 6 files changed, 30 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
>> index d840d75..ded3f20 100644
>> --- a/include/linux/sysctl.h
>> +++ b/include/linux/sysctl.h
>> @@ -426,6 +426,7 @@ enum
>> NET_TCP_MAX_SSTHRESH=124,
>> NET_TCP_FRTO_RESPONSE=125,
>> NET_TCP_FORCE_THIN_LINEAR_TIMEOUTS=126,
>> + NET_TCP_FORCE_THIN_LINEAR_DUPACK=127,
>
> There is no need to allocate a binary sysctl here.
>
> Eric
Thanks. I'll address this in the next iteration.
Best regrads,
Andreas
^ permalink raw reply
* Re: [net-next PATCH v3 2/3] net: TCP thin linear timeouts
From: Andreas Petlund @ 2010-02-13 15:50 UTC (permalink / raw)
To: William Allen Simpson
Cc: netdev@vger.kernel.org, Ilpo Järvinen, Eric Dumazet,
Arnd Hannemann, LKML, shemminger, David Miller, damian
In-Reply-To: <4B753931.30405@gmail.com>
On 12. feb. 2010 12:19, William Allen Simpson wrote:
> Last year, I'm pretty sure I was on record as thinking this is only a
> marginally good idea, that would be better at the application layer.
>
> Also that naming was a bit dicey. Now the names are more descriptive,
> but the "force" is a bit overkill.
>
> How about?
> NET_TCP_FORCE_THIN_LINEAR_TIMEOUTS -> NET_TCP_THIN_LINEAR_TIMEOUTS
> TCP_THIN_LT -> TCP_THIN_LINEAR_TIMEOUTS
> TCP_THIN_LT_RETRIES -> TCP_THIN_LINEAR_RETRIES
> tcp_force_thin_linear_timeouts -> tcp_thin_linear_timeouts
> sysctl_tcp_force_thin_linear_timeouts -> sysctl_tcp_thin_linear_timeouts
> tp->thin_lt -> tp->thin_lto
>
> The latter mostly traditional "to" for "timeout", as used most everywhere.
>
I agree that the _force_-part should be taken out for both patches, and
renaming the lt to lto also makes sense. I'll fix it in the next iteration.
> Just for efficiency, I'd reorder this
> + if (sk->sk_state == TCP_ESTABLISHED &&
> + (tp->thin_lt || sysctl_tcp_force_thin_linear_timeouts) &&
> + tcp_stream_is_thin(sk) &&
> + icsk->icsk_retransmits <= TCP_THIN_LT_RETRIES) {
Thank you for this suggestion. I'll reorder in the next iteration.
Best regards,
Andreas
^ permalink raw reply
* Re: [net-next PATCH v3 2/3] net: TCP thin linear timeouts
From: Andreas Petlund @ 2010-02-13 15:49 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev@vger.kernel.org, Ilpo Järvinen, Arnd Hannemann, LKML,
shemminger, David Miller, william.allen.simpson, damian
In-Reply-To: <1265946733.2891.10.camel@edumazet-laptop>
On 12. feb. 2010 04:52, Eric Dumazet wrote:
>
> Hi Anreas
>
> Could you include a section in Documentation/networking/ip-sysctl.txt
> about tcp_force_thin_linear_timeouts setting ?
>
I 'll submit a draft with the next iteration. thanks for the heads up on the procedure.
It may also be helpful with a separate textfile to describe the thin-stream latency
issues and the mechanisms. I will submit a draft for this also.
> Also, you should provide some documentation to be included in man pages
> (man 7 tcp), to Michael Kerrisk ( <mtk.manpages@gmail.com> ), both for
> tcp_force_thin_linear_timeouts and TCP_THIN_LT
>
> (Same applies for patch 3/3 and tcp_force_thin_dupack /
> TCP_THIN_DUPACK )
>
I'll make a draft, and mail him as soon as the naming issues are settled.
Best regards,
Andreas
^ permalink raw reply
* Re: [net-next PATCH v3 3/3] net: TCP thin dupack
From: Andreas Petlund @ 2010-02-13 15:50 UTC (permalink / raw)
To: William Allen Simpson
Cc: netdev@vger.kernel.org, Ilpo Järvinen, Eric Dumazet,
Arnd Hannemann, LKML, shemminger, David Miller, damian
In-Reply-To: <4B753943.2080800@gmail.com>
On 12. feb. 2010 12:19, William Allen Simpson wrote:
> Last year, I'm pretty sure I was on record as thinking this is *not* a
> good idea. But at least it now requires a sysctl to turn on, and
> should default to off.
>
> Also that naming was a bit dicey. Now the names are more descriptive,
> but the "force" is a bit overkill.
>
> How about:
> NET_TCP_FORCE_THIN_LINEAR_DUPACK -> NET_TCP_THIN_LINEAR_DUPACK
> tcp_force_thin_dupack -> tcp_thin_linear_dupack
> sysctl_tcp_force_thin_dupack -> sysctl_tcp_thin_linear_dupack
You uncovered a copy/paste/edit-typo there. The term "linear" had snuck
in even though it does not make sense for this patch. I think that
NET_TCP_THIN_DUPACK, tcp_thin_dupack and sysctl_tcp_thin_dupack will
be better.
Best regards,
Andreas
^ permalink raw reply
* Re: [PATCH] iputils: ping by mark
From: jamal @ 2010-02-13 16:21 UTC (permalink / raw)
To: YOSHIFUJI Hideaki; +Cc: netdev
In-Reply-To: <4B76C976.2060708@linux-ipv6.org>
[-- Attachment #1: Type: text/plain, Size: 194 bytes --]
On Sun, 2010-02-14 at 00:47 +0900, YOSHIFUJI Hideaki wrote:
> Hello.
>
> Okay, applied. Thank you.
> Could you give me patch for doc/ping.sgml as well, please?
Ok, here it is..
cheers,
jamal
[-- Attachment #2: ping.sgml-mark --]
[-- Type: text/plain, Size: 1342 bytes --]
commit 6e063d914a97c4b8160cb43a3129ea368b2a3fbb
Author: Jamal Hadi Salim <hadi@cyberus.ca>
Date: Sat Feb 13 11:19:13 2010 -0500
iputils: ping by mark doc update
Update documentation for ping by-mark
Signed-off-by: Jamal Hadi Salim <hadi@cyberus.ca>
diff --git a/doc/ping.sgml b/doc/ping.sgml
index 5995ea9..c895d7d 100644
--- a/doc/ping.sgml
+++ b/doc/ping.sgml
@@ -16,6 +16,7 @@
<command>ping</command>
<arg choice="opt"><option>-LRUbdfnqrvVaAB</option></arg>
<arg choice="opt">-c <replaceable/count/</arg>
+<arg choice="opt">-m <replaceable/mark/</arg>
<arg choice="opt">-i <replaceable/interval/</arg>
<arg choice="opt">-l <replaceable/preload/</arg>
<arg choice="opt">-p <replaceable/pattern/</arg>
@@ -76,6 +77,14 @@ The address is bound to one selected when <command/ping/ starts.
</para></listitem>
</varlistentry>
<varlistentry>
+ <term><option>-m <replaceable/mark/</option></term>
+ <listitem><para>
+use <replaceable/mark/ to tag the packets going out. This is useful
+for variety of reasons within the kernel such as using policy
+routing to select specific outbound processing.
+ </para></listitem>
+ </varlistentry>
+ <varlistentry>
<term><option><anchor id="ping.count">-c <replaceable/count/</option></term>
<listitem><para>
Stop after sending <replaceable/count/ ECHO_REQUEST
^ permalink raw reply related
* Re: [PATCH net-next-2.6] macvtap: Add GSO/csum offload support
From: Arnd Bergmann @ 2010-02-13 17:34 UTC (permalink / raw)
To: Sridhar Samudrala; +Cc: David Miller, Herbert Xu, netdev
In-Reply-To: <1266044316.10419.3.camel@w-sridhar.beaverton.ibm.com>
Hi Sridhar,
On Saturday 13 February 2010, Sridhar Samudrala wrote:
> This patch adds GSO/checksum offload support to macvtap driver and applies
> on top of Arnd's refcnt bugfix.
> http://patchwork.ozlabs.org/patch/45136/
Sorry for messing this up by replacing that patch with a different one.
It shouldn't be hard to rebase this one though, which I'll probably do on Monday.
Please tell me if you want to do it yourself instead.
> @@ -286,6 +288,7 @@ static int macvtap_open(struct inode *inode, struct file *file)
> sock_init_data(&q->sock, &q->sk);
> q->sk.sk_allocation = GFP_ATOMIC; /* for now */
> q->sk.sk_write_space = macvtap_sock_write_space;
> + q->flags = IFF_VNET_HDR;
>
> err = macvtap_set_queue(dev, file, q);
> if (err)
Making IFF_VNET_HDR the default probably prevents the driver from working
with applications that don't known about VNET_HDR, e.g. anything other
than qemu. I believe qemu always tries setting it though, which would make
a default value of !IFF_NET_HDR fine.
Also, what about IFF_TAP and IFF_NO_PI, should those be always set?
> @@ -499,18 +648,14 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
> return 0;
>
> case TUNSETOFFLOAD:
> - /* let the user check for future flags */
> - if (arg & ~(TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 |
> - TUN_F_TSO_ECN | TUN_F_UFO))
> - return -EINVAL;
> -
> - /* TODO: add support for these, so far we don't
> - support any offload */
> - if (arg & (TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 |
> - TUN_F_TSO_ECN | TUN_F_UFO))
> - return -EINVAL;
> -
> - return 0;
> + q = macvtap_file_get_queue(file);
> + if (!q)
> + return -ENOLINK;
> + ret = 0;
> + if (!(q->flags & IFF_VNET_HDR))
> + ret = -EINVAL;
> + macvtap_file_put_queue(q);
> + return ret;
>
> default:
> return -EINVAL;
At least the first check needs to be in there, in case we are running with
new user space that knows additional flags. Moreover, shouldn't we check
the flags against the capabilities of vlan->lowerdev? I though it would be
best to report the capabilities of the real hardware to the guest kernel
so it can do the right thing.
Arnd
^ permalink raw reply
* Re: [PATCH] iputils: ping by mark
From: YOSHIFUJI Hideaki @ 2010-02-13 17:35 UTC (permalink / raw)
To: hadi; +Cc: netdev, YOSHIFUJI Hideaki
In-Reply-To: <1266078076.6776.40.camel@bigi>
Applied. Thanks.
jamal wrote:
> On Sun, 2010-02-14 at 00:47 +0900, YOSHIFUJI Hideaki wrote:
>> Hello.
>>
>> Okay, applied. Thank you.
>> Could you give me patch for doc/ping.sgml as well, please?
>
> Ok, here it is..
>
> cheers,
> jamal
>
^ permalink raw reply
* Re: [PATCH] vhost-net: switch to smp barriers
From: Michael S. Tsirkin @ 2010-02-13 17:39 UTC (permalink / raw)
To: Rusty Russell; +Cc: kvm, virtualization, netdev, linux-kernel, David Miller
In-Reply-To: <201002081849.39512.rusty@rustcorp.com.au>
On Mon, Feb 08, 2010 at 06:49:39PM +1030, Rusty Russell wrote:
> On Sun, 7 Feb 2010 07:37:49 pm Michael S. Tsirkin wrote:
> > On Mon, Feb 01, 2010 at 07:21:02PM +0200, Michael S. Tsirkin wrote:
> > > vhost-net only uses memory barriers to control SMP effects
> > > (communication with userspace potentially running on a different CPU),
> > > so it should use SMP barriers and not mandatory barriers for memory
> > > access ordering, as suggested by Documentation/memory-barriers.txt
> > >
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >
> >
> > Rusty, any feedback on this one?
> > Thanks!
>
> Yep. barrier() is correct on UP to guard against preemption.
>
> Acked-by: Rusty Russell <rusty@rustcorp.com.au>
>
> Thanks,
> Rusty.
Dave, I see it's marked "not applicable":
http://patchwork.ozlabs.org/patch/44207/
the patch applies to net-next as of
b3b3f04fb587ecb61b5baa6c1c5f0e666fd12d73.
Can this be queued up please?
Should I resubmit with Rusty's ack?
Thanks!
--
MST
^ permalink raw reply
* [RFC]: IPv4 mroute interface referencing
From: David Lamparter @ 2010-02-13 18:29 UTC (permalink / raw)
To: netdev
[-- Attachment #1: Type: text/plain, Size: 1210 bytes --]
The attached patch introduces a new struct vifctln which is identical to
vifctl except for a new vifi_ifindex field. This allows creating mroute
vifs if you have multiple interfaces with the same IP (i.e.
"unnumbered", PtP interfaces) on your box.
Other ways to support this could be:
* new mroute API
+ it really needs an overhaul anyway
+ netlink would be more appropriate for mroute (IMHO)
- complicated, non-immediate fix
- complex user space software rewriting
* new MRT_ADD_VIF_EXT setsockopt
+ a bit more clean
- not needed really, sockopt mess
* stuff ifindex in lcl_addr field ("0.0.0.ifindex IP")
+ no new struct field
- idiotic (IMHO) API-monging
o compatibility gain if BSD did this, but BSD doesn't, no point
* add ifindex field (attached patch does this)
+ compatibility by setsockopt length parameter
+ doesn't break userspace software (none of the existing multicast
routers tries to pass excess junk)
- new struct, new struct field
This patch causes userspace-visible changes, but is fully backwards
compatible. Patch is against 2.6.32.8, I will provide an updated one if
discussion is in favour of something I can code up (i.e. not netlink).
Regards,
David
[-- Attachment #2: Type: text/x-patch, Size: 4419 bytes --]
>From 93e78e1d65976643ea8de54cfbe46f5741f49a9d Mon Sep 17 00:00:00 2001
From: David Lamparter <equinox@diac24.net>
Date: Sat, 13 Feb 2010 19:19:22 +0100
Subject: [PATCH] ipmr: new struct vifctln to support ifindex for MIF_ADD_VIF
it isn't currently possible to do multicast routing if multiple
interfaces with the same IP address are present in a system, as only the
first of them can be selected for mroute VIF creation.
fix this by adding struct vifctln (analogous to struct mreqn) which has
a new int vifi_ifindex field. the setsockopt call differentiates vifctl
from vifctln through the length field; non-present or zero vifi_ifindex
keeps the old behaviour, nonzero vifi_ifindex causes vifi_lcladdr to be
ignored in favour of vifi_ifindex.
Signed-off-by: David Lamparter <equinox@diac24.net>
---
include/linux/mroute.h | 14 ++++++++++++++
net/ipv4/ipmr.c | 19 +++++++++++++------
2 files changed, 27 insertions(+), 6 deletions(-)
diff --git a/include/linux/mroute.h b/include/linux/mroute.h
index 08bc776..0ef73ce 100644
--- a/include/linux/mroute.h
+++ b/include/linux/mroute.h
@@ -63,6 +63,20 @@ struct vifctl {
struct in_addr vifc_rmt_addr; /* IPIP tunnel addr */
};
+/* Linux extension: reference VIF by ifindex.
+ * this struct MUST exactly retain the fields from vifctl for compatibility!
+ * vifc_ifindex is used if nonzero, otherwise fall back to old behaviour
+ */
+struct vifctln {
+ vifi_t vifc_vifi; /* Index of VIF */
+ unsigned char vifc_flags; /* VIFF_ flags */
+ unsigned char vifc_threshold; /* ttl limit */
+ unsigned int vifc_rate_limit; /* Rate limiter values (NI) */
+ struct in_addr vifc_lcl_addr; /* Our address */
+ struct in_addr vifc_rmt_addr; /* IPIP tunnel addr */
+ int vifc_ifindex; /* Index of real Interface */
+};
+
#define VIFF_TUNNEL 0x1 /* IPIP tunnel */
#define VIFF_SRCRT 0x2 /* NI */
#define VIFF_REGISTER 0x4 /* register vif */
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 99508d6..9d5d22e 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -23,6 +23,7 @@
* Carlos Picoto : PIMv1 Support
* Pavlin Ivanov Radoslavov: PIMv2 Registers must checksum only PIM header
* Relax this requrement to work with older peers.
+ * David Lamparter : struct vifctln introduced
*
*/
@@ -135,7 +136,7 @@ static void ipmr_del_tunnel(struct net_device *dev, struct vifctl *v)
}
static
-struct net_device *ipmr_new_tunnel(struct net *net, struct vifctl *v)
+struct net_device *ipmr_new_tunnel(struct net *net, struct vifctln *v)
{
struct net_device *dev;
@@ -149,6 +150,7 @@ struct net_device *ipmr_new_tunnel(struct net *net, struct vifctl *v)
struct in_device *in_dev;
memset(&p, 0, sizeof(p));
+ p.link = v->vifc_ifindex;
p.iph.daddr = v->vifc_rmt_addr.s_addr;
p.iph.saddr = v->vifc_lcl_addr.s_addr;
p.iph.version = 4;
@@ -426,7 +428,7 @@ static void ipmr_update_thresholds(struct mfc_cache *cache, unsigned char *ttls)
}
}
-static int vif_add(struct net *net, struct vifctl *vifc, int mrtsock)
+static int vif_add(struct net *net, struct vifctln *vifc, int mrtsock)
{
int vifi = vifc->vifc_vifi;
struct vif_device *v = &net->ipv4.vif_table[vifi];
@@ -470,7 +472,10 @@ static int vif_add(struct net *net, struct vifctl *vifc, int mrtsock)
}
break;
case 0:
- dev = ip_dev_find(net, vifc->vifc_lcl_addr.s_addr);
+ if (vifc->vifc_ifindex)
+ dev = dev_get_by_index(net, vifc->vifc_ifindex);
+ else
+ dev = ip_dev_find(net, vifc->vifc_lcl_addr.s_addr);
if (!dev)
return -EADDRNOTAVAIL;
err = dev_set_allmulti(dev, 1);
@@ -936,7 +941,7 @@ static void mrtsock_destruct(struct sock *sk)
int ip_mroute_setsockopt(struct sock *sk, int optname, char __user *optval, unsigned int optlen)
{
int ret;
- struct vifctl vif;
+ struct vifctln vif;
struct mfcctl mfc;
struct net *net = sock_net(sk);
@@ -975,9 +980,11 @@ int ip_mroute_setsockopt(struct sock *sk, int optname, char __user *optval, unsi
return ip_ra_control(sk, 0, NULL);
case MRT_ADD_VIF:
case MRT_DEL_VIF:
- if (optlen != sizeof(vif))
+ /* either vifctl or vifctln is fine; take what we get */
+ if (optlen != sizeof(vif) && optlen != sizeof(struct vifctl))
return -EINVAL;
- if (copy_from_user(&vif, optval, sizeof(vif)))
+ vif.vifc_ifindex = 0;
+ if (copy_from_user(&vif, optval, optlen))
return -EFAULT;
if (vif.vifc_vifi >= MAXVIFS)
return -ENFILE;
--
1.6.5.2
^ permalink raw reply related
* [PATCH] p54usb: Add usbid for Corega CG-WLUSB2GT.
From: YOSHIFUJI Hideaki @ 2010-02-13 19:16 UTC (permalink / raw)
To: linux-wireless, netdev, linville, David Miller; +Cc: yoshfuji, hirofumi
From: Shimada Hirofumi <hirofumi@flycat.org>
p54usb: Add usbid for Corega CG-WLUSB2GT.
Signed-off-by: Shimada Hirofumi <hirofumi@flycat.org>
Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
drivers/net/wireless/p54/p54usb.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wireless/p54/p54usb.c b/drivers/net/wireless/p54/p54usb.c
index 92af9b9..dcb484b 100644
--- a/drivers/net/wireless/p54/p54usb.c
+++ b/drivers/net/wireless/p54/p54usb.c
@@ -36,6 +36,7 @@ static struct usb_device_id p54u_table[] __devinitdata = {
/* Version 1 devices (pci chip + net2280) */
{USB_DEVICE(0x0506, 0x0a11)}, /* 3COM 3CRWE254G72 */
{USB_DEVICE(0x0707, 0xee06)}, /* SMC 2862W-G */
+ {USB_DEVICE(0x07aa, 0x001c)}, /* Corega CG-WLUSB2GT */
{USB_DEVICE(0x083a, 0x4501)}, /* Accton 802.11g WN4501 USB */
{USB_DEVICE(0x083a, 0x4502)}, /* Siemens Gigaset USB Adapter */
{USB_DEVICE(0x083a, 0x5501)}, /* Phillips CPWUA054 */
^ permalink raw reply related
* [PATCH net-next-2.6 1/2] cxgb3: FIx VLAN over Jumbo frames
From: Divy Le Ray @ 2010-02-13 19:44 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-kernel, swise
From: Divy Le Ray <divy@chelsio.com>
The mac is expected to auto-inflate the Maximum Frame size for VLAN
tagged frames. It however does not work with jumbo frames.
Work around the bug adding 4 to the Maximum Frame for MTUs
greater than 1536.
Signed-off-by: Divy Le Ray <divy@chelsio.com>
---
drivers/net/cxgb3/xgmac.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/net/cxgb3/xgmac.c b/drivers/net/cxgb3/xgmac.c
index 0109ee4..0c08de5 100644
--- a/drivers/net/cxgb3/xgmac.c
+++ b/drivers/net/cxgb3/xgmac.c
@@ -353,6 +353,9 @@ int t3_mac_set_mtu(struct cmac *mac, unsigned int mtu)
* packet size register includes header, but not FCS.
*/
mtu += 14;
+ if (mtu > 1536)
+ mtu += 4;
+
if (mtu > MAX_FRAME_SIZE - 4)
return -EINVAL;
t3_write_reg(adap, A_XGM_RX_MAX_PKT_SIZE + mac->offset, mtu);
^ permalink raw reply related
* [PATCH net-next-2.6 2/2] cxgb3: fix link flap
From: Divy Le Ray @ 2010-02-13 19:44 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-kernel, swise
In-Reply-To: <20100213194429.27297.99280.stgit@speedy5.asicdesigners.com>
From: Divy Le Ray <divy@chelsio.com>
The driver is expected to report that the link is up
when the phy Rx signal is established and the mac
has not detected a link fault.
The code is however broken, the driver does not check the link fault
status when the phy link status changes.
The link fault status being checked within a short period of time,
it leads to link up/link down events.
Signed-off-by: Divy Le Ray <divy@chelsio.com>
---
drivers/net/cxgb3/t3_hw.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/net/cxgb3/t3_hw.c b/drivers/net/cxgb3/t3_hw.c
index 032cfe0..3ab9f51 100644
--- a/drivers/net/cxgb3/t3_hw.c
+++ b/drivers/net/cxgb3/t3_hw.c
@@ -1262,7 +1262,8 @@ void t3_link_changed(struct adapter *adapter, int port_id)
lc->fc = fc;
}
- t3_os_link_changed(adapter, port_id, link_ok, speed, duplex, fc);
+ t3_os_link_changed(adapter, port_id, link_ok && !pi->link_fault,
+ speed, duplex, fc);
}
void t3_link_fault(struct adapter *adapter, int port_id)
^ permalink raw reply related
* Re: [PATCH net-next-2.6] macvtap: Add GSO/csum offload support
From: Sridhar Samudrala @ 2010-02-13 20:55 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: David Miller, Herbert Xu, netdev
In-Reply-To: <201002131834.00612.arnd@arndb.de>
On 2/13/2010 9:34 AM, Arnd Bergmann wrote:
> Hi Sridhar,
>
> On Saturday 13 February 2010, Sridhar Samudrala wrote:
>
>
>> This patch adds GSO/checksum offload support to macvtap driver and applies
>> on top of Arnd's refcnt bugfix.
>> http://patchwork.ozlabs.org/patch/45136/
>>
> Sorry for messing this up by replacing that patch with a different one.
> It shouldn't be hard to rebase this one though, which I'll probably do on Monday.
> Please tell me if you want to do it yourself instead.
>
No problem. If you get to it before, it is fine with me.
>> @@ -286,6 +288,7 @@ static int macvtap_open(struct inode *inode, struct file *file)
>> sock_init_data(&q->sock,&q->sk);
>> q->sk.sk_allocation = GFP_ATOMIC; /* for now */
>> q->sk.sk_write_space = macvtap_sock_write_space;
>> + q->flags = IFF_VNET_HDR;
>>
>> err = macvtap_set_queue(dev, file, q);
>> if (err)
>>
> Making IFF_VNET_HDR the default probably prevents the driver from working
> with applications that don't known about VNET_HDR, e.g. anything other
> than qemu. I believe qemu always tries setting it though, which would make
> a default value of !IFF_NET_HDR fine.
>
Yes. It is better to make the default as !IFF_VNET_HDR
> Also, what about IFF_TAP and IFF_NO_PI, should those be always set?
>
Atleast it is not required for qemu to have these flags set. If we are
not doing anything different based on
these flags, i felt we don't need to have them.
>
>> @@ -499,18 +648,14 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
>> return 0;
>>
>> case TUNSETOFFLOAD:
>> - /* let the user check for future flags */
>> - if (arg& ~(TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 |
>> - TUN_F_TSO_ECN | TUN_F_UFO))
>> - return -EINVAL;
>> -
>> - /* TODO: add support for these, so far we don't
>> - support any offload */
>> - if (arg& (TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 |
>> - TUN_F_TSO_ECN | TUN_F_UFO))
>> - return -EINVAL;
>> -
>> - return 0;
>> + q = macvtap_file_get_queue(file);
>> + if (!q)
>> + return -ENOLINK;
>> + ret = 0;
>> + if (!(q->flags& IFF_VNET_HDR))
>> + ret = -EINVAL;
>> + macvtap_file_put_queue(q);
>> + return ret;
>>
>> default:
>> return -EINVAL;
>>
> At least the first check needs to be in there, in case we are running with
> new user space that knows additional flags. Moreover, shouldn't we check
> the flags against the capabilities of vlan->lowerdev? I though it would be
> best to report the capabilities of the real hardware to the guest kernel
> so it can do the right thing.
>
Originally, i also thought we should check these based on the real
device capabilities. But later i realized,
that it is not really required as we fall back to software offload via
dev_gso_segment() call in dev_hard_start_xmit()
if the real device doesn't support any of the offloads. So we can
advertise that all the offloads are supported to
the guest and let host deal with any offloads that are not supported by
the real device.
Thanks
Sridhar
^ permalink raw reply
* [PATCH] atm : fix /sys/devices/virtual/atm/X/carrier(ATM_PHY_SIG_UNKNOWN)
From: Philippe De Muyter @ 2010-02-13 21:56 UTC (permalink / raw)
To: chas, netdev
When trying to design udev rules to automate usage of cxacru usb atm (adsl)
modem, I have discovered that /sys/devices/virtual/atm/cxacru0/carrier had the
value '1' while actually carrier was not yet established and real carrier
state was not yet known to linux. I propose to fix that by using '?' as the
/sys/devices/virtual/atm/cxacru0/carrier value when carrier state is not yet
known to linux. Any other value except '1' would also be OK for me.
--
Currently, just after the interface creation,
/sys/devices/virtual/atm/cxacru0/carrier gives wrong info (carrier = 1),
while actually carrier is unknown. Fix that.
Signed-off-by: Philippe De Muyter <phdm@macqel.be>
--- a/net/atm/atm_sysfs.c 2010-02-13 22:54:04.598847195 +0100
+++ b/net/atm/atm_sysfs.c 2010-02-13 19:13:45.395308531 +0100
@@ -63,9 +63,9 @@ static ssize_t show_carrier(struct devic
char *pos = buf;
struct atm_dev *adev = to_atm_dev(cdev);
- pos += sprintf(pos, "%d\n",
- adev->signal ==
- adev->signal == ATM_PHY_SIG_LOST ? 0 : 1);
+ pos += sprintf(pos, "%c\n",
+ adev->signal == ATM_PHY_SIG_FOUND ? '1' :
+ adev->signal == ATM_PHY_SIG_LOST ? '0' : '?');
return pos - buf;
}
^ permalink raw reply
* Re: [PATCH] atm : fix /sys/devices/virtual/atm/X/carrier(ATM_PHY_SIG_UNKNOWN)
From: Chas Williams (CONTRACTOR) @ 2010-02-13 23:24 UTC (permalink / raw)
To: Philippe De Muyter; +Cc: netdev
In-Reply-To: <20100213215633.GA4345@frolo.macqel>
In message <20100213215633.GA4345@frolo.macqel>,Philippe De Muyter writes:
>value '1' while actually carrier was not yet established and real carrier
>state was not yet known to linux. I propose to fix that by using '?' as the
>/sys/devices/virtual/atm/cxacru0/carrier value when carrier state is not yet
>known to linux. Any other value except '1' would also be OK for me.
this is sort of intentional because some drivers dont actually implement
atm_dev->signal. ATM_PHY_SIG_UNKNOWN and ATM_PHY_SIG_LOST should
likely be carrier = 0. if the driver doesnt isnt going to handle
changing the state of atm_dev->signal it should just set the value to
ATM_PHY_SIG_FOUND.
i dont like the idea of carrier being '?' -- carrier is either true or false.
you have it or you dont.
^ permalink raw reply
* [PATCH 0/7] tcp: bugs and cleanup updated to 2.6.33-rc8
From: William Allen Simpson @ 2010-02-14 6:01 UTC (permalink / raw)
To: Linux Kernel Developers, Linux Kernel Network Developers
Cc: Andrew Morton, David Miller
Combination of patches reported in October, November, December, January
and February. These patches fix perceived bugs, and other cleanup.
This code has had previous review and several months of limited testing.
Some portions were removed during the various TCPCT part 1 patch splits,
then were cut off by the sudden unexpected end of that merge window.
[03 Dec 2009] I've restarted the sub-numbering (again).
Of particular interest are the TCPCT header extensions that already
appear in the next phase of testing with other platforms. These patches
allow correct reception without data corruption.
The remainder of the original TCPCT part 2 will be merged with part 3.
These are patches against the current linux-2.6 tree.
[Parts 2f and 2g updated, improving resistance to very rare option
re-ordering by middleware.]
^ permalink raw reply
* [PATCH v3 1/7] net: tcp_header_len_th and tcp_option_len_th
From: William Allen Simpson @ 2010-02-14 6:05 UTC (permalink / raw)
To: Linux Kernel Developers, Linux Kernel Network Developers
Cc: Andrew Morton, David Miller
In-Reply-To: <4B7791CF.9030201@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 359 bytes --]
Redefine two TCP header functions to accept TCP header pointer.
When subtracting, return signed int to allow error checking.
These functions will be used in subsequent patches that implement
additional features.
Signed-off-by: William.Allen.Simpson@gmail.com
---
include/linux/tcp.h | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
[-- Attachment #2: len_th+2a3+2.6.33-rc8.patch --]
[-- Type: text/plain, Size: 719 bytes --]
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index 7fee8a4..d0133cf 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -223,6 +223,18 @@ static inline unsigned int tcp_optlen(const struct sk_buff *skb)
return (tcp_hdr(skb)->doff - 5) * 4;
}
+/* Length of fixed header plus standard options. */
+static inline unsigned int tcp_header_len_th(const struct tcphdr *th)
+{
+ return th->doff * 4;
+}
+
+/* Length of standard options only. This could be negative. */
+static inline int tcp_option_len_th(const struct tcphdr *th)
+{
+ return (int)(th->doff * 4) - sizeof(*th);
+}
+
/* This defines a selective acknowledgement block. */
struct tcp_sack_block_wire {
__be32 start_seq;
--
1.6.3.3
^ permalink raw reply related
* [PATCH v3 2/7] net: remove old tcp_optlen function
From: William Allen Simpson @ 2010-02-14 6:09 UTC (permalink / raw)
To: Linux Kernel Developers, Linux Kernel Network Developers
Cc: Andrew Morton, David Miller, Michael Chan
In-Reply-To: <4B7791CF.9030201@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1070 bytes --]
The tcp_optlen() function returns a potential *negative* unsigned.
In the only two existing files using the old tcp_optlen() function,
clean up confusing and inconsistent mixing of both byte and word
offsets, and other coding style issues. Document assumptions.
Quoth David Miller:
This is transmit, and the packets can only come from the Linux
TCP stack, not some external entity.
You're being way too anal here, and adding these checks to
drivers would be just a lot of rediculious bloat. [sic]
Therefore, there are *no* checks for bad TCP and IP header sizes, nor
any semantic changes. The drivers should function exactly as existing.
No response from testers in 17+ weeks.
Requires:
net: tcp_header_len_th and tcp_option_len_th
Signed-off-by: William.Allen.Simpson@gmail.com
CC: Michael Chan <mchan@broadcom.com>
---
drivers/net/bnx2.c | 29 +++++++++++++-----------
drivers/net/tg3.c | 60 +++++++++++++++++++++++---------------------------
include/linux/tcp.h | 5 ----
3 files changed, 44 insertions(+), 50 deletions(-)
[-- Attachment #2: len_th+2b3+2.6.33-rc8.patch --]
[-- Type: text/plain, Size: 6936 bytes --]
diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index 65df1de..45452c5 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -6352,6 +6352,8 @@ bnx2_vlan_rx_register(struct net_device *dev, struct vlan_group *vlgrp)
/* Called with netif_tx_lock.
* bnx2_tx_int() runs without netif_tx_lock unless it needs to call
* netif_wake_queue().
+ *
+ * No TCP or IP length checking, per David Miller (see commit log).
*/
static netdev_tx_t
bnx2_start_xmit(struct sk_buff *skb, struct net_device *dev)
@@ -6396,19 +6398,19 @@ bnx2_start_xmit(struct sk_buff *skb, struct net_device *dev)
(TX_BD_FLAGS_VLAN_TAG | (vlan_tx_tag_get(skb) << 16));
}
#endif
- if ((mss = skb_shinfo(skb)->gso_size)) {
- u32 tcp_opt_len;
- struct iphdr *iph;
+ mss = skb_shinfo(skb)->gso_size;
+ if (mss != 0) {
+ struct tcphdr *th = tcp_hdr(skb);
+ int tcp_opt_words = th->doff - (sizeof(*th) >> 2);
+ /* assumes positive tcp_opt_words without checking */
vlan_tag_flags |= TX_BD_FLAGS_SW_LSO;
- tcp_opt_len = tcp_optlen(skb);
-
if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6) {
u32 tcp_off = skb_transport_offset(skb) -
sizeof(struct ipv6hdr) - ETH_HLEN;
- vlan_tag_flags |= ((tcp_opt_len >> 2) << 8) |
+ vlan_tag_flags |= (tcp_opt_words << 8) |
TX_BD_FLAGS_SW_FLAGS;
if (likely(tcp_off == 0))
vlan_tag_flags &= ~TX_BD_FLAGS_TCP6_OFF0_MSK;
@@ -6421,14 +6423,15 @@ bnx2_start_xmit(struct sk_buff *skb, struct net_device *dev)
mss |= (tcp_off & 0xc) << TX_BD_TCP6_OFF2_SHL;
}
} else {
- iph = ip_hdr(skb);
- if (tcp_opt_len || (iph->ihl > 5)) {
- vlan_tag_flags |= ((iph->ihl - 5) +
- (tcp_opt_len >> 2)) << 8;
- }
+ struct iphdr *iph = ip_hdr(skb);
+ int ip_opt_words = iph->ihl - (sizeof(*iph) >> 2);
+ /* assumes positive ip_opt_words without checking */
+ int opt_words = ip_opt_words + tcp_opt_words;
+
+ if (opt_words > 0)
+ vlan_tag_flags |= opt_words << 8;
}
- } else
- mss = 0;
+ }
mapping = pci_map_single(bp->pdev, skb->data, len, PCI_DMA_TODEVICE);
if (pci_dma_mapping_error(bp->pdev, mapping)) {
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 7f82b02..c20c800 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -5426,6 +5426,8 @@ static void tg3_set_txd(struct tg3_napi *tnapi, int entry,
/* hard_start_xmit for devices that don't have any bugs and
* support TG3_FLG2_HW_TSO_2 and TG3_FLG2_HW_TSO_3 only.
+ *
+ * No TCP or IP length checking, per David Miller (see commit log).
*/
static netdev_tx_t tg3_start_xmit(struct sk_buff *skb,
struct net_device *dev)
@@ -5461,9 +5463,9 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb,
entry = tnapi->tx_prod;
base_flags = 0;
- mss = 0;
- if ((mss = skb_shinfo(skb)->gso_size) != 0) {
- int tcp_opt_len, ip_tcp_len;
+ mss = skb_shinfo(skb)->gso_size;
+ if (mss != 0) {
+ struct tcphdr *th;
u32 hdrlen;
if (skb_header_cloned(skb) &&
@@ -5471,18 +5473,16 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb,
dev_kfree_skb(skb);
goto out_unlock;
}
+ th = tcp_hdr(skb);
if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6)
hdrlen = skb_headlen(skb) - ETH_HLEN;
else {
struct iphdr *iph = ip_hdr(skb);
- tcp_opt_len = tcp_optlen(skb);
- ip_tcp_len = ip_hdrlen(skb) + sizeof(struct tcphdr);
-
+ hdrlen = ip_hdrlen(skb) + tcp_header_len_th(th);
+ iph->tot_len = htons(mss + hdrlen);
iph->check = 0;
- iph->tot_len = htons(mss + ip_tcp_len + tcp_opt_len);
- hdrlen = ip_tcp_len + tcp_opt_len;
}
if (tp->tg3_flags2 & TG3_FLG2_HW_TSO_3) {
@@ -5496,7 +5496,7 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb,
base_flags |= (TXD_FLAG_CPU_PRE_DMA |
TXD_FLAG_CPU_POST_DMA);
- tcp_hdr(skb)->check = 0;
+ th->check = 0;
}
else if (skb->ip_summed == CHECKSUM_PARTIAL)
@@ -5629,6 +5629,8 @@ tg3_tso_bug_end:
/* hard_start_xmit for devices that have the 4G bug and/or 40-bit bug and
* support TG3_FLG2_HW_TSO_1 or firmware TSO only.
+ *
+ * No TCP or IP length checking, per David Miller (see commit log).
*/
static netdev_tx_t tg3_start_xmit_dma_bug(struct sk_buff *skb,
struct net_device *dev)
@@ -5668,20 +5670,21 @@ static netdev_tx_t tg3_start_xmit_dma_bug(struct sk_buff *skb,
if (skb->ip_summed == CHECKSUM_PARTIAL)
base_flags |= TXD_FLAG_TCPUDP_CSUM;
- if ((mss = skb_shinfo(skb)->gso_size) != 0) {
+ mss = skb_shinfo(skb)->gso_size;
+ if (mss != 0) {
struct iphdr *iph;
- u32 tcp_opt_len, ip_tcp_len, hdr_len;
+ struct tcphdr *th;
+ u32 hdr_len;
+ int opt_bytes;
if (skb_header_cloned(skb) &&
pskb_expand_head(skb, 0, 0, GFP_ATOMIC)) {
dev_kfree_skb(skb);
goto out_unlock;
}
+ th = tcp_hdr(skb);
+ hdr_len = ip_hdrlen(skb) + tcp_header_len_th(th);
- tcp_opt_len = tcp_optlen(skb);
- ip_tcp_len = ip_hdrlen(skb) + sizeof(struct tcphdr);
-
- hdr_len = ip_tcp_len + tcp_opt_len;
if (unlikely((ETH_HLEN + hdr_len) > 80) &&
(tp->tg3_flags2 & TG3_FLG2_TSO_BUG))
return (tg3_tso_bug(tp, skb));
@@ -5693,13 +5696,14 @@ static netdev_tx_t tg3_start_xmit_dma_bug(struct sk_buff *skb,
iph->check = 0;
iph->tot_len = htons(mss + hdr_len);
if (tp->tg3_flags2 & TG3_FLG2_HW_TSO) {
- tcp_hdr(skb)->check = 0;
+ th->check = 0;
base_flags &= ~TXD_FLAG_TCPUDP_CSUM;
} else
- tcp_hdr(skb)->check = ~csum_tcpudp_magic(iph->saddr,
- iph->daddr, 0,
- IPPROTO_TCP,
- 0);
+ th->check = ~csum_tcpudp_magic(iph->saddr, iph->daddr,
+ 0, IPPROTO_TCP, 0);
+
+ opt_bytes = hdr_len - sizeof(*iph) - sizeof(*th);
+ /* assumes positive opt_bytes without checking */
if (tp->tg3_flags2 & TG3_FLG2_HW_TSO_3) {
mss |= (hdr_len & 0xc) << 12;
@@ -5710,19 +5714,11 @@ static netdev_tx_t tg3_start_xmit_dma_bug(struct sk_buff *skb,
mss |= hdr_len << 9;
else if ((tp->tg3_flags2 & TG3_FLG2_HW_TSO_1) ||
GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5705) {
- if (tcp_opt_len || iph->ihl > 5) {
- int tsflags;
-
- tsflags = (iph->ihl - 5) + (tcp_opt_len >> 2);
- mss |= (tsflags << 11);
- }
+ if (opt_bytes > 0)
+ mss |= opt_bytes << (11 - 2);
} else {
- if (tcp_opt_len || iph->ihl > 5) {
- int tsflags;
-
- tsflags = (iph->ihl - 5) + (tcp_opt_len >> 2);
- base_flags |= tsflags << 12;
- }
+ if (opt_bytes > 0)
+ base_flags |= opt_bytes << (12 - 2);
}
}
#if TG3_VLAN_TAG_USED
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index d0133cf..74728f7 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -218,11 +218,6 @@ static inline unsigned int tcp_hdrlen(const struct sk_buff *skb)
return tcp_hdr(skb)->doff * 4;
}
-static inline unsigned int tcp_optlen(const struct sk_buff *skb)
-{
- return (tcp_hdr(skb)->doff - 5) * 4;
-}
-
/* Length of fixed header plus standard options. */
static inline unsigned int tcp_header_len_th(const struct tcphdr *th)
{
--
1.6.3.3
^ permalink raw reply related
* [PATCH v5 3/7] tcp: harmonize tcp_vx_rcv header length assumptions
From: William Allen Simpson @ 2010-02-14 6:16 UTC (permalink / raw)
To: Linux Kernel Developers, Linux Kernel Network Developers
Cc: Andrew Morton, David Miller, Andi Kleen
In-Reply-To: <4B7791CF.9030201@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 825 bytes --]
Harmonize tcp_v4_rcv() and tcp_v6_rcv() -- better document tcp doff
and header length assumptions, and carefully compare implementations.
Reduces multiply/shifts, marginally improving speed.
Removes redundant tcp header length checks before checksumming.
Instead, assumes (and documents) that any backlog processing and
transform policies will carefully preserve the header, and will
ensure the socket buffer length remains >= the header size.
Stand-alone patch, originally developed for TCPCT.
Signed-off-by: William.Allen.Simpson@gmail.com
CC: Andi Kleen <andi@firstfloor.org>
---
include/net/xfrm.h | 7 ++++++
net/ipv4/tcp_ipv4.c | 45 +++++++++++++++++++++-----------------
net/ipv6/tcp_ipv6.c | 59 ++++++++++++++++++++++++++++----------------------
3 files changed, 65 insertions(+), 46 deletions(-)
[-- Attachment #2: len_th+2c5+2.6.33-rc8.patch --]
[-- Type: text/plain, Size: 8037 bytes --]
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 60c2770..81492a1 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -975,6 +975,13 @@ xfrm_state_addr_cmp(struct xfrm_tmpl *tmpl, struct xfrm_state *x, unsigned short
}
#ifdef CONFIG_XFRM
+/*
+ * For transport, the policy is checked before the presumed more expensive
+ * checksum. The transport header has already been checked for size, and is
+ * guaranteed to be contiguous. These policies must not alter the header or
+ * its position in the buffer, and should not shorten the buffer length
+ * without ensuring the length remains >= the header size.
+ */
extern int __xfrm_policy_check(struct sock *, int dir, struct sk_buff *skb, unsigned short family);
static inline int __xfrm_policy_check2(struct sock *sk, int dir,
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 65b8ebf..0a76e41 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1559,7 +1559,8 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
return 0;
}
- if (skb->len < tcp_hdrlen(skb) || tcp_checksum_complete(skb))
+ /* Assumes header and options unchanged since checksum_init() */
+ if (tcp_checksum_complete(skb))
goto csum_err;
if (sk->sk_state == TCP_LISTEN) {
@@ -1601,14 +1602,13 @@ csum_err:
}
/*
- * From tcp_input.c
+ * Called by ip_input.c: ip_local_deliver_finish()
*/
-
int tcp_v4_rcv(struct sk_buff *skb)
{
- const struct iphdr *iph;
struct tcphdr *th;
struct sock *sk;
+ int tcp_header_len;
int ret;
struct net *net = dev_net(skb->dev);
@@ -1618,31 +1618,33 @@ int tcp_v4_rcv(struct sk_buff *skb)
/* Count it even if it's bad */
TCP_INC_STATS_BH(net, TCP_MIB_INSEGS);
+ /* Check too short header */
if (!pskb_may_pull(skb, sizeof(struct tcphdr)))
goto discard_it;
- th = tcp_hdr(skb);
-
- if (th->doff < sizeof(struct tcphdr) / 4)
+ /* Check bad doff, compare doff directly to constant value */
+ tcp_header_len = tcp_hdr(skb)->doff;
+ if (tcp_header_len < (sizeof(struct tcphdr) / 4))
goto bad_packet;
- if (!pskb_may_pull(skb, th->doff * 4))
+
+ /* Check too short header and options */
+ tcp_header_len *= 4;
+ if (!pskb_may_pull(skb, tcp_header_len))
goto discard_it;
- /* An explanation is required here, I think.
- * Packet length and doff are validated by header prediction,
- * provided case of th->doff==0 is eliminated.
- * So, we defer the checks. */
+ /* Packet length and doff are validated by header prediction,
+ * provided case of th->doff == 0 is eliminated (above).
+ */
if (!skb_csum_unnecessary(skb) && tcp_v4_checksum_init(skb))
goto bad_packet;
th = tcp_hdr(skb);
- iph = ip_hdr(skb);
TCP_SKB_CB(skb)->seq = ntohl(th->seq);
TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th->syn + th->fin +
- skb->len - th->doff * 4);
+ skb->len - tcp_header_len);
TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
TCP_SKB_CB(skb)->when = 0;
- TCP_SKB_CB(skb)->flags = iph->tos;
+ TCP_SKB_CB(skb)->flags = ip_hdr(skb)->tos;
TCP_SKB_CB(skb)->sacked = 0;
sk = __inet_lookup_skb(&tcp_hashinfo, skb, th->source, th->dest);
@@ -1682,14 +1684,14 @@ process:
bh_unlock_sock(sk);
sock_put(sk);
-
return ret;
no_tcp_socket:
if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb))
goto discard_it;
- if (skb->len < (th->doff << 2) || tcp_checksum_complete(skb)) {
+ /* Assumes header and options unchanged since checksum_init() */
+ if (tcp_checksum_complete(skb)) {
bad_packet:
TCP_INC_STATS_BH(net, TCP_MIB_INERRS);
} else {
@@ -1711,18 +1713,21 @@ do_time_wait:
goto discard_it;
}
- if (skb->len < (th->doff << 2) || tcp_checksum_complete(skb)) {
+ /* Assumes header and options unchanged since checksum_init() */
+ if (tcp_checksum_complete(skb)) {
TCP_INC_STATS_BH(net, TCP_MIB_INERRS);
inet_twsk_put(inet_twsk(sk));
goto discard_it;
}
+
switch (tcp_timewait_state_process(inet_twsk(sk), skb, th)) {
case TCP_TW_SYN: {
struct sock *sk2 = inet_lookup_listener(dev_net(skb->dev),
&tcp_hashinfo,
- iph->daddr, th->dest,
+ ip_hdr(skb)->daddr,
+ th->dest,
inet_iif(skb));
- if (sk2) {
+ if (sk2 != NULL) {
inet_twsk_deschedule(inet_twsk(sk), &tcp_death_row);
inet_twsk_put(inet_twsk(sk));
sk = sk2;
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index febfd59..b76939a 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1594,7 +1594,8 @@ static int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
return 0;
}
- if (skb->len < tcp_hdrlen(skb) || tcp_checksum_complete(skb))
+ /* Assumes header and options unchanged since checksum_init() */
+ if (tcp_checksum_complete(skb))
goto csum_err;
if (sk->sk_state == TCP_LISTEN) {
@@ -1664,38 +1665,47 @@ ipv6_pktoptions:
return 0;
}
+/*
+ * Called by ip6_input.c: ip6_input_finish()
+ */
static int tcp_v6_rcv(struct sk_buff *skb)
{
struct tcphdr *th;
struct sock *sk;
+ int tcp_header_len;
int ret;
struct net *net = dev_net(skb->dev);
if (skb->pkt_type != PACKET_HOST)
goto discard_it;
- /*
- * Count it even if it's bad.
- */
+ /* Count it even if it's bad */
TCP_INC_STATS_BH(net, TCP_MIB_INSEGS);
+ /* Check too short header */
if (!pskb_may_pull(skb, sizeof(struct tcphdr)))
goto discard_it;
- th = tcp_hdr(skb);
-
- if (th->doff < sizeof(struct tcphdr)/4)
+ /* Check bad doff, compare doff directly to constant value */
+ tcp_header_len = tcp_hdr(skb)->doff;
+ if (tcp_header_len < (sizeof(struct tcphdr) / 4))
goto bad_packet;
- if (!pskb_may_pull(skb, th->doff*4))
+
+ /* Check too short header and options */
+ tcp_header_len *= 4;
+ if (!pskb_may_pull(skb, tcp_header_len))
goto discard_it;
+ /* Packet length and doff are validated by header prediction,
+ * provided case of th->doff == 0 is eliminated (above).
+ */
if (!skb_csum_unnecessary(skb) && tcp_v6_checksum_init(skb))
goto bad_packet;
th = tcp_hdr(skb);
TCP_SKB_CB(skb)->seq = ntohl(th->seq);
TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th->syn + th->fin +
- skb->len - th->doff*4);
+ skb->len - tcp_header_len);
TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
TCP_SKB_CB(skb)->when = 0;
TCP_SKB_CB(skb)->flags = ipv6_get_dsfield(ipv6_hdr(skb));
@@ -1711,6 +1721,7 @@ process:
if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb))
goto discard_and_relse;
+ /* nf_reset(skb); in ip6_input.c ip6_input_finish() */
if (sk_filter(sk, skb))
goto discard_and_relse;
@@ -1743,7 +1754,8 @@ no_tcp_socket:
if (!xfrm6_policy_check(NULL, XFRM_POLICY_IN, skb))
goto discard_it;
- if (skb->len < (th->doff<<2) || tcp_checksum_complete(skb)) {
+ /* Assumes header and options unchanged since checksum_init() */
+ if (tcp_checksum_complete(skb)) {
bad_packet:
TCP_INC_STATS_BH(net, TCP_MIB_INERRS);
} else {
@@ -1751,11 +1763,7 @@ bad_packet:
}
discard_it:
-
- /*
- * Discard frame
- */
-
+ /* Discard frame. */
kfree_skb(skb);
return 0;
@@ -1769,24 +1777,23 @@ do_time_wait:
goto discard_it;
}
- if (skb->len < (th->doff<<2) || tcp_checksum_complete(skb)) {
+ /* Assumes header and options unchanged since checksum_init() */
+ if (tcp_checksum_complete(skb)) {
TCP_INC_STATS_BH(net, TCP_MIB_INERRS);
inet_twsk_put(inet_twsk(sk));
goto discard_it;
}
switch (tcp_timewait_state_process(inet_twsk(sk), skb, th)) {
- case TCP_TW_SYN:
- {
- struct sock *sk2;
-
- sk2 = inet6_lookup_listener(dev_net(skb->dev), &tcp_hashinfo,
- &ipv6_hdr(skb)->daddr,
- ntohs(th->dest), inet6_iif(skb));
+ case TCP_TW_SYN: {
+ struct sock *sk2 = inet6_lookup_listener(dev_net(skb->dev),
+ &tcp_hashinfo,
+ &ipv6_hdr(skb)->daddr,
+ ntohs(th->dest),
+ inet6_iif(skb));
if (sk2 != NULL) {
- struct inet_timewait_sock *tw = inet_twsk(sk);
- inet_twsk_deschedule(tw, &tcp_death_row);
- inet_twsk_put(tw);
+ inet_twsk_deschedule(inet_twsk(sk), &tcp_death_row);
+ inet_twsk_put(inet_twsk(sk));
sk = sk2;
goto process;
}
--
1.6.3.3
^ permalink raw reply related
* Re: [Bug #15042] socket(PF_INET6 hangs when ipv6 not yet initialized
From: Marc Haber @ 2010-02-14 6:20 UTC (permalink / raw)
To: Américo Wang
Cc: Rafael J. Wysocki, Linux Kernel Mailing List, Kernel Testers List,
Linux Kernel Network Developers
In-Reply-To: <2375c9f91001270110t2a8d08b8n4578df310a6129@mail.gmail.com>
Hi,
sorry for the late reply.
On Wed, Jan 27, 2010 at 05:10:44PM +0800, Américo Wang wrote:
> On Mon, Jan 25, 2010 at 6:23 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > This message has been generated automatically as a part of a report
> > of regressions introduced between 2.6.31 and 2.6.32.
> >
> > The following bug entry is on the current list of known regressions
> > introduced between 2.6.31 and 2.6.32. Please verify if it still should
> > be listed and let me know (either way).
> >
> >
> > Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=15042
> > Subject : socket(PF_INET6 hangs when ipv6 not yet initialized
> > Submitter : Marc Haber <mh+linux-kernel@zugschlus.de>
> > Date : 2010-01-10 14:28 (15 days old)
> > References : http://marc.info/?l=linux-kernel&m=126313553029280&w=4
> >
>
> (Adding net-dev into Cc)
>
> Hi, Marc,
>
> Sorry, I don't really understand your description of the problem, your mean
> your ssh hangs on socket PF_INET6 for 30 seconds?
No, it hangs on socket PF_INET6 indefinetely (max time I waited was
like ten minutes), ignoring SIGTERM while hanging. This does only
happen if the sshd is started within seconds of the ipv6 module being
loaded (or the Interface being brought up), so I guess that the kernel
needs to be in a state where it already thinks that it has IPv6 but
hasn't completed autoconfiguration yet.
To obtain an strace, I had to use timeout --signal=9 30. The
virtualbox process this happens inside takes all host CPU while the
ssh process hangs.
> Have you enabled CONFIG_DETECT_SOFTLOCKUP for your kernel?
yes, that's enabled.
Greetings
Marc
--
-----------------------------------------------------------------------------
Marc Haber | "I don't trust Computers. They | Mailadresse im Header
Mannheim, Germany | lose things." Winona Ryder | Fon: *49 621 72739834
Nordisch by Nature | How to make an American Quilt | Fax: *49 3221 2323190
Bitte beachten Sie, daß dem [m.E. grundgesetzwidrigen] Gesetz zur
Vorratsdatenspeicherung zufolge, seit dem 1. Januar 2008 jeglicher
elektronische Kontakt (E-Mail, Telefongespräche, SMS, Internet-
Telefonie, Mobilfunk, Fax) mit mir oder anderen Nutzern verdachts-
unabhängig für den automatisierten geheimen Zugriff durch Strafver-
folgungs- u. Polizeivollzugsbehörden, die Bundesanstalt für Finanz-
dienstleistungsaufsicht, Zollkriminal- und Zollfahndungsämter,die
Zollverwaltung zur Schwarzarbeitsbekämpfung, Notrufabfragestellen,
Verfassungsschutzbehörden, den Militärischen Abschirmdienst, Bundes-
nachrichtendienst sowie 52 Staaten wie beispielsweise Aserbeidschan
oder die USA sechs Monate lang gespeichert wird, einschließlich der
Kommunikation mit Berufsgeheimnisträgern wie Ärzten, Journalisten und
Anwälten. Mehr Infos zur totalen Protokollierung Ihrer Kommunikations-
daten auf www.vorratsdatenspeicherung.de. (leicht verändert übernommen
kopiert von www.lawblog.de)
^ permalink raw reply
* [PATCH v4 4/7] tcp: input header length, prediction, and timestamp bugs
From: William Allen Simpson @ 2010-02-14 6:21 UTC (permalink / raw)
To: Linux Kernel Developers, Linux Kernel Network Developers
Cc: Andrew Morton, David Miller, Andi Kleen
In-Reply-To: <4B7791CF.9030201@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1871 bytes --]
Fix incorrect header prediction flags documentation.
Relieve register pressure in (the i386) fast path by accessing skb->len
directly, instead of carrying a rarely used len parameter.
Eliminate unused len parameters in two other functions.
Don't use output calculated tp->tcp_header_len for input decisions.
While the output header is usually the same as the input (same options
in both directions), that's a poor assumption. In particular, Sack will
be different. Newer options are not guaranteed.
Moreover, in the fast path, that only saved a shift or two. The other
efficiencies in this patch more than make up the difference.
Instead, use tp->rx_opt.tstamp_ok to accurately predict header length.
Likewise, use tp->rx_opt.tstamp_ok for received MSS calculations.
Don't use "sizeof(struct tcphdr) + TCPOLEN_TSTAMP_ALIGNED" to guess that
the timestamp is present. This may have been OK in the days with fewer
possible options, but various combinations of newer options may yield
the same header length. (This bug is in 3 places.)
Instead, use tp->rx_opt.saw_tstamp to determine a timestamp is present.
There's no need to test buffer length against header length, already
checked by tcp_v[4,6]_rcv(). Straighten code for minor efficiency gain.
Stand-alone patch, originally developed for TCPCT.
Requires:
net: tcp_header_len_th and tcp_option_len_th
tcp: harmonize tcp_vx_rcv header length assumptions
Signed-off-by: William.Allen.Simpson@gmail.com
CC: Andi Kleen <andi@firstfloor.org>
---
include/linux/tcp.h | 6 ++-
include/net/tcp.h | 15 +++++--
net/ipv4/tcp_input.c | 94 ++++++++++++++++++----------------------------
net/ipv4/tcp_ipv4.c | 4 +-
net/ipv4/tcp_minisocks.c | 3 +-
net/ipv4/tcp_probe.c | 2 +-
net/ipv6/tcp_ipv6.c | 4 +-
7 files changed, 58 insertions(+), 70 deletions(-)
[-- Attachment #2: len_th+2d4+2.6.33-rc8.patch --]
[-- Type: text/plain, Size: 9932 bytes --]
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index 74728f7..2987ee8 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -301,7 +301,11 @@ struct tcp_sock {
/*
* Header prediction flags
- * 0x5?10 << 16 + snd_wnd in net byte order
+ * S << 28 + TCP_FLAG_ACK + snd_wnd, in net byte order
+ * (PSH flag is ignored)
+ * S is 5 (no options), or 8 (timestamp aligned)
+ * otherwise, 0 to turn it off -- for instance, when there are
+ * holes in receive space.
*/
__be32 pred_flags;
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 34f5cc2..6b0d7e9 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -310,13 +310,11 @@ extern int tcp_ioctl(struct sock *sk,
extern int tcp_rcv_state_process(struct sock *sk,
struct sk_buff *skb,
- struct tcphdr *th,
- unsigned len);
+ struct tcphdr *th);
extern int tcp_rcv_established(struct sock *sk,
struct sk_buff *skb,
- struct tcphdr *th,
- unsigned len);
+ struct tcphdr *th);
extern void tcp_rcv_space_adjust(struct sock *sk);
@@ -533,9 +531,16 @@ static inline u32 __tcp_set_rto(const struct tcp_sock *tp)
return (tp->srtt >> 3) + tp->rttvar;
}
+static inline u16 __tcp_fast_path_header_length(const struct tcp_sock *tp)
+{
+ return tp->rx_opt.tstamp_ok
+ ? sizeof(struct tcphdr) + TCPOLEN_TSTAMP_ALIGNED
+ : sizeof(struct tcphdr);
+}
+
static inline void __tcp_fast_path_on(struct tcp_sock *tp, u32 snd_wnd)
{
- tp->pred_flags = htonl((tp->tcp_header_len << 26) |
+ tp->pred_flags = htonl((__tcp_fast_path_header_length(tp) << (28 - 2)) |
ntohl(TCP_FLAG_ACK) |
snd_wnd);
}
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 28e0296..8e0f6ae 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -152,7 +152,7 @@ static void tcp_measure_rcv_mss(struct sock *sk, const struct sk_buff *skb)
* tcp header plus fixed timestamp option length.
* Resulting "len" is MSS free of SACK jitter.
*/
- len -= tcp_sk(sk)->tcp_header_len;
+ len -= __tcp_fast_path_header_length(tcp_sk(sk));
icsk->icsk_ack.last_seg_size = len;
if (len == lss) {
icsk->icsk_ack.rcv_mss = len;
@@ -5206,7 +5206,7 @@ discard:
* tcp_data_queue when everything is OK.
*/
int tcp_rcv_established(struct sock *sk, struct sk_buff *skb,
- struct tcphdr *th, unsigned len)
+ struct tcphdr *th)
{
struct tcp_sock *tp = tcp_sk(sk);
int res;
@@ -5225,31 +5225,15 @@ int tcp_rcv_established(struct sock *sk, struct sk_buff *skb,
* extra cost of the net_bh soft interrupt processing...
* We do checksum and copy also but from device to kernel.
*/
-
- tp->rx_opt.saw_tstamp = 0;
-
- /* pred_flags is 0xS?10 << 16 + snd_wnd
- * if header_prediction is to be made
- * 'S' will always be tp->tcp_header_len >> 2
- * '?' will be 0 for the fast path, otherwise pred_flags is 0 to
- * turn it off (when there are holes in the receive
- * space for instance)
- * PSH flag is ignored.
- */
-
if ((tcp_flag_word(th) & TCP_HP_BITS) == tp->pred_flags &&
TCP_SKB_CB(skb)->seq == tp->rcv_nxt &&
!after(TCP_SKB_CB(skb)->ack_seq, tp->snd_nxt)) {
- int tcp_header_len = tp->tcp_header_len;
-
- /* Timestamp header prediction: tcp_header_len
- * is automatically equal to th->doff*4 due to pred_flags
- * match.
- */
+ int tcp_header_len = tcp_header_len_th(th);
- /* Check timestamp */
- if (tcp_header_len == sizeof(struct tcphdr) + TCPOLEN_TSTAMP_ALIGNED) {
- /* No? Slow path! */
+ /* Timestamp header prediction */
+ if (tcp_header_len != sizeof(*th) + TCPOLEN_TSTAMP_ALIGNED) {
+ tp->rx_opt.saw_tstamp = 0; /* false */
+ } else {
if (!tcp_parse_aligned_timestamp(tp, th))
goto slow_path;
@@ -5264,35 +5248,12 @@ int tcp_rcv_established(struct sock *sk, struct sk_buff *skb,
*/
}
- if (len <= tcp_header_len) {
- /* Bulk data transfer: sender */
- if (len == tcp_header_len) {
- /* Predicted packet is in window by definition.
- * seq == rcv_nxt and rcv_wup <= rcv_nxt.
- * Hence, check seq<=rcv_wup reduces to:
- */
- if (tcp_header_len ==
- (sizeof(struct tcphdr) + TCPOLEN_TSTAMP_ALIGNED) &&
- tp->rcv_nxt == tp->rcv_wup)
- tcp_store_ts_recent(tp);
-
- /* We know that such packets are checksummed
- * on entry.
- */
- tcp_ack(sk, skb, 0);
- __kfree_skb(skb);
- tcp_data_snd_check(sk);
- return 0;
- } else { /* Header too small */
- TCP_INC_STATS_BH(sock_net(sk), TCP_MIB_INERRS);
- goto discard;
- }
- } else {
+ if (tcp_header_len < skb->len) {
int eaten = 0;
int copied_early = 0;
if (tp->copied_seq == tp->rcv_nxt &&
- len - tcp_header_len <= tp->ucopy.len) {
+ skb->len - tcp_header_len <= tp->ucopy.len) {
#ifdef CONFIG_NET_DMA
if (tcp_dma_try_early_copy(sk, skb, tcp_header_len)) {
copied_early = 1;
@@ -5311,9 +5272,7 @@ int tcp_rcv_established(struct sock *sk, struct sk_buff *skb,
* seq == rcv_nxt and rcv_wup <= rcv_nxt.
* Hence, check seq<=rcv_wup reduces to:
*/
- if (tcp_header_len ==
- (sizeof(struct tcphdr) +
- TCPOLEN_TSTAMP_ALIGNED) &&
+ if (tp->rx_opt.saw_tstamp &&
tp->rcv_nxt == tp->rcv_wup)
tcp_store_ts_recent(tp);
@@ -5334,8 +5293,7 @@ int tcp_rcv_established(struct sock *sk, struct sk_buff *skb,
* seq == rcv_nxt and rcv_wup <= rcv_nxt.
* Hence, check seq<=rcv_wup reduces to:
*/
- if (tcp_header_len ==
- (sizeof(struct tcphdr) + TCPOLEN_TSTAMP_ALIGNED) &&
+ if (tp->rx_opt.saw_tstamp &&
tp->rcv_nxt == tp->rcv_wup)
tcp_store_ts_recent(tp);
@@ -5376,11 +5334,33 @@ no_ack:
else
sk->sk_data_ready(sk, 0);
return 0;
+ } else {
+ /* Bulk data transfer: sender
+ *
+ * tcp_header_len > skb->len never happens,
+ * already checked by tcp_v[4,6]_rcv()
+ *
+ * Predicted packet is in window by definition.
+ * seq == rcv_nxt and rcv_wup <= rcv_nxt.
+ * Hence, check seq<=rcv_wup reduces to:
+ */
+ if (tp->rx_opt.saw_tstamp &&
+ tp->rcv_nxt == tp->rcv_wup)
+ tcp_store_ts_recent(tp);
+
+ /* We know that such packets are checksummed
+ * on entry.
+ */
+ tcp_ack(sk, skb, 0);
+ __kfree_skb(skb);
+ tcp_data_snd_check(sk);
+ return 0;
}
}
slow_path:
- if (len < (th->doff << 2) || tcp_checksum_complete_user(sk, skb))
+ /* Assumes header and options unchanged since checksum_init() */
+ if (tcp_checksum_complete_user(sk, skb))
goto csum_error;
/*
@@ -5416,7 +5396,7 @@ discard:
}
static int tcp_rcv_synsent_state_process(struct sock *sk, struct sk_buff *skb,
- struct tcphdr *th, unsigned len)
+ struct tcphdr *th)
{
u8 *hash_location;
struct inet_connection_sock *icsk = inet_csk(sk);
@@ -5693,7 +5673,7 @@ reset_and_undo:
*/
int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb,
- struct tcphdr *th, unsigned len)
+ struct tcphdr *th)
{
struct tcp_sock *tp = tcp_sk(sk);
struct inet_connection_sock *icsk = inet_csk(sk);
@@ -5740,7 +5720,7 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb,
goto discard;
case TCP_SYN_SENT:
- queued = tcp_rcv_synsent_state_process(sk, skb, th, len);
+ queued = tcp_rcv_synsent_state_process(sk, skb, th);
if (queued >= 0)
return queued;
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 0a76e41..f999e06 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1551,7 +1551,7 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
if (sk->sk_state == TCP_ESTABLISHED) { /* Fast path */
TCP_CHECK_TIMER(sk);
- if (tcp_rcv_established(sk, skb, tcp_hdr(skb), skb->len)) {
+ if (tcp_rcv_established(sk, skb, tcp_hdr(skb))) {
rsk = sk;
goto reset;
}
@@ -1578,7 +1578,7 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
}
TCP_CHECK_TIMER(sk);
- if (tcp_rcv_state_process(sk, skb, tcp_hdr(skb), skb->len)) {
+ if (tcp_rcv_state_process(sk, skb, tcp_hdr(skb))) {
rsk = sk;
goto reset;
}
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index f206ee5..37b7536 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -718,8 +718,7 @@ int tcp_child_process(struct sock *parent, struct sock *child,
int state = child->sk_state;
if (!sock_owned_by_user(child)) {
- ret = tcp_rcv_state_process(child, skb, tcp_hdr(skb),
- skb->len);
+ ret = tcp_rcv_state_process(child, skb, tcp_hdr(skb));
/* Wakeup parent, send SIGIO */
if (state == TCP_SYN_RECV && child->sk_state != state)
parent->sk_data_ready(parent, 0);
diff --git a/net/ipv4/tcp_probe.c b/net/ipv4/tcp_probe.c
index 9bc805d..de2a32e 100644
--- a/net/ipv4/tcp_probe.c
+++ b/net/ipv4/tcp_probe.c
@@ -88,7 +88,7 @@ static inline int tcp_probe_avail(void)
* Note: arguments must match tcp_rcv_established()!
*/
static int jtcp_rcv_established(struct sock *sk, struct sk_buff *skb,
- struct tcphdr *th, unsigned len)
+ struct tcphdr *th)
{
const struct tcp_sock *tp = tcp_sk(sk);
const struct inet_sock *inet = inet_sk(sk);
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index b76939a..3d08a4d 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1586,7 +1586,7 @@ static int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
if (sk->sk_state == TCP_ESTABLISHED) { /* Fast path */
TCP_CHECK_TIMER(sk);
- if (tcp_rcv_established(sk, skb, tcp_hdr(skb), skb->len))
+ if (tcp_rcv_established(sk, skb, tcp_hdr(skb)))
goto reset;
TCP_CHECK_TIMER(sk);
if (opt_skb)
@@ -1618,7 +1618,7 @@ static int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
}
TCP_CHECK_TIMER(sk);
- if (tcp_rcv_state_process(sk, skb, tcp_hdr(skb), skb->len))
+ if (tcp_rcv_state_process(sk, skb, tcp_hdr(skb)))
goto reset;
TCP_CHECK_TIMER(sk);
if (opt_skb)
--
1.6.3.3
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox