* Re: Recent select() handling change breaks Poptop [not found] ` <20041207045302.GA23746@linuxace.com> @ 2004-12-07 5:48 ` Mitchell Blank Jr 2004-12-07 15:08 ` [PATCH] fix select() for SOCK_RAW sockets Mitchell Blank Jr 0 siblings, 1 reply; 10+ messages in thread From: Mitchell Blank Jr @ 2004-12-07 5:48 UTC (permalink / raw) To: Phil Oester; +Cc: shemminger, linux-net, linux-kernel, netdev (adding netdev to cc:) Phil Oester wrote: > > 2. a "tcpdump -nvv" of its udp traffic (ideally captured from a seperate > > server, but from the server would probably be OK too) > > PPTP uses TCP 1723 and GRE (proto 47), so there is no udp traffic involved. > I suspect the change was made to all datagram traffic with the assumption > that UDP was the only protocol impacted. Perhaps GRE was not considered? Yeah, it looks like the problem for sure. The patch modifies the structure "inet_dgram_ops" to use udp_poll(), but looking farther down: static struct inet_protosw inetsw_array[] = [...] .type = SOCK_DGRAM, .protocol = IPPROTO_UDP, .prot = &udp_prot, .ops = &inet_dgram_ops, [...] .type = SOCK_RAW, .protocol = IPPROTO_IP, /* wild card */ .prot = &raw_prot, .ops = &inet_dgram_ops, [...] so it looks like udp_poll() will end up getting used for both SOCK_DGRAM and SOCK_RAW inet sockets; obviously Poptop is using the latter and failing as a result. No need for the strace/tcpdump data I guess. The fix is to just make a copy of the inet_dgram_ops called inet_udp_ops and make the udp_poll() change only in that one (and obviously change the SOCK_DGRAM case there to use &inet_udp_ops). I don't have time right this second to spin a patch, but could you try that out and see if it fixes your problem. -Mitch ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] fix select() for SOCK_RAW sockets 2004-12-07 5:48 ` Recent select() handling change breaks Poptop Mitchell Blank Jr @ 2004-12-07 15:08 ` Mitchell Blank Jr 2004-12-07 17:28 ` Phil Oester ` (3 more replies) 0 siblings, 4 replies; 10+ messages in thread From: Mitchell Blank Jr @ 2004-12-07 15:08 UTC (permalink / raw) To: Phil Oester, David S. Miller, shemminger; +Cc: linux-net, linux-kernel, netdev Phil: Here's a real patch for you to test. I actually left inet_dgram_ops alone since it's an exported symbol (two of the users just want the .do_ioctl value which is the same between SOCK_DGRAM and SOCK_RAW; the other is ipv6 where it's clearly dealing with a UDP socket -- therefore I think its safest to leave inet_dgram_ops to have the UDP behavior) Davem: I only tested that this doesn't break UDP; if it works for Phil and Stephen can verify that it doesn't break his bad-checksum UDP tests then please push it for 2.6.10. Patch is versus 2.6.10-rc3. Signed-off-by: Mitchell Blank Jr <mitch@sfgoth.com> -Mitch --- linux-2.6.10-rc3-VIRGIN/net/ipv4/af_inet.c 2004-12-07 06:37:52.480082706 -0800 +++ linux-2.6.10-rc3/net/ipv4/af_inet.c 2004-12-07 06:57:47.799013216 -0800 @@ -821,6 +821,31 @@ .sendpage = inet_sendpage, }; +/* + * For SOCK_RAW sockets; should be the same as inet_dgram_ops but without + * udp_poll + */ +static struct proto_ops inet_sockraw_ops = { + .family = PF_INET, + .owner = THIS_MODULE, + .release = inet_release, + .bind = inet_bind, + .connect = inet_dgram_connect, + .socketpair = sock_no_socketpair, + .accept = sock_no_accept, + .getname = inet_getname, + .poll = datagram_poll, + .ioctl = inet_ioctl, + .listen = sock_no_listen, + .shutdown = inet_shutdown, + .setsockopt = sock_common_setsockopt, + .getsockopt = sock_common_getsockopt, + .sendmsg = inet_sendmsg, + .recvmsg = sock_common_recvmsg, + .mmap = sock_no_mmap, + .sendpage = inet_sendpage, +}; + static struct net_proto_family inet_family_ops = { .family = PF_INET, .create = inet_create, @@ -861,7 +886,7 @@ .type = SOCK_RAW, .protocol = IPPROTO_IP, /* wild card */ .prot = &raw_prot, - .ops = &inet_dgram_ops, + .ops = &inet_sockraw_ops, .capability = CAP_NET_RAW, .no_check = UDP_CSUM_DEFAULT, .flags = INET_PROTOSW_REUSE, ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] fix select() for SOCK_RAW sockets 2004-12-07 15:08 ` [PATCH] fix select() for SOCK_RAW sockets Mitchell Blank Jr @ 2004-12-07 17:28 ` Phil Oester 2004-12-07 17:35 ` YOSHIFUJI Hideaki / 吉藤英明 ` (2 subsequent siblings) 3 siblings, 0 replies; 10+ messages in thread From: Phil Oester @ 2004-12-07 17:28 UTC (permalink / raw) To: Mitchell Blank Jr Cc: David S. Miller, shemminger, linux-net, linux-kernel, netdev On Tue, Dec 07, 2004 at 07:08:34AM -0800, Mitchell Blank Jr wrote: > Phil: Here's a real patch for you to test. I actually left inet_dgram_ops > alone since it's an exported symbol (two of the users just want the .do_ioctl > value which is the same between SOCK_DGRAM and SOCK_RAW; the other is ipv6 > where it's clearly dealing with a UDP socket -- therefore I think its safest > to leave inet_dgram_ops to have the UDP behavior) > > Davem: I only tested that this doesn't break UDP; if it works for Phil and > Stephen can verify that it doesn't break his bad-checksum UDP tests then > please push it for 2.6.10. Yup, that does indeed fix it for me, thanks. Phil ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] fix select() for SOCK_RAW sockets 2004-12-07 15:08 ` [PATCH] fix select() for SOCK_RAW sockets Mitchell Blank Jr 2004-12-07 17:28 ` Phil Oester @ 2004-12-07 17:35 ` YOSHIFUJI Hideaki / 吉藤英明 2004-12-07 18:01 ` [PATCH] fix select() for SOCK_RAW sockets (ipv6) Stephen Hemminger 2004-12-07 17:45 ` [PATCH] fix select() for SOCK_RAW sockets Stephen Hemminger 2004-12-08 5:28 ` David S. Miller 3 siblings, 1 reply; 10+ messages in thread From: YOSHIFUJI Hideaki / 吉藤英明 @ 2004-12-07 17:35 UTC (permalink / raw) To: mitch; +Cc: kernel, davem, shemminger, linux-net, linux-kernel, netdev, yoshfuji In article <20041207150834.GA75700@gaz.sfgoth.com> (at Tue, 7 Dec 2004 07:08:34 -0800), Mitchell Blank Jr <mitch@sfgoth.com> says: > Phil: Here's a real patch for you to test. I actually left inet_dgram_ops > alone since it's an exported symbol (two of the users just want the .do_ioctl > value which is the same between SOCK_DGRAM and SOCK_RAW; the other is ipv6 > where it's clearly dealing with a UDP socket -- therefore I think its safest > to leave inet_dgram_ops to have the UDP behavior) Probably, we need to do the same for ipv6, don't we? --yoshfuji ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] fix select() for SOCK_RAW sockets (ipv6) 2004-12-07 17:35 ` YOSHIFUJI Hideaki / 吉藤英明 @ 2004-12-07 18:01 ` Stephen Hemminger 2004-12-07 18:48 ` David S. Miller 0 siblings, 1 reply; 10+ messages in thread From: Stephen Hemminger @ 2004-12-07 18:01 UTC (permalink / raw) To: YOSHIFUJI Hideaki / ____________ Cc: mitch, kernel, davem, linux-net, linux-kernel, netdev, yoshfuji > Probably, we need to do the same for ipv6, don't we? diff -Nru a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c --- a/net/ipv6/af_inet6.c 2004-12-07 10:02:50 -08:00 +++ b/net/ipv6/af_inet6.c 2004-12-07 10:02:50 -08:00 @@ -513,6 +513,27 @@ .sendpage = sock_no_sendpage, }; +struct proto_ops inet6_raw_ops = { + .family = PF_INET6, + .owner = THIS_MODULE, + .release = inet6_release, + .bind = inet6_bind, + .connect = inet_dgram_connect, /* ok */ + .socketpair = sock_no_socketpair, /* a do nothing */ + .accept = sock_no_accept, /* a do nothing */ + .getname = inet6_getname, + .poll = datagram_poll, /* ok */ + .ioctl = inet6_ioctl, /* must change */ + .listen = sock_no_listen, /* ok */ + .shutdown = inet_shutdown, /* ok */ + .setsockopt = sock_common_setsockopt, /* ok */ + .getsockopt = sock_common_getsockopt, /* ok */ + .sendmsg = inet_sendmsg, /* ok */ + .recvmsg = sock_common_recvmsg, /* ok */ + .mmap = sock_no_mmap, + .sendpage = sock_no_sendpage, +}; + static struct net_proto_family inet6_family_ops = { .family = PF_INET6, .create = inet6_create, @@ -528,7 +549,7 @@ .type = SOCK_RAW, .protocol = IPPROTO_IP, /* wild card */ .prot = &rawv6_prot, - .ops = &inet6_dgram_ops, + .ops = &inet6_raw_ops, .capability = CAP_NET_RAW, .no_check = UDP_CSUM_DEFAULT, .flags = INET_PROTOSW_REUSE, ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] fix select() for SOCK_RAW sockets (ipv6) 2004-12-07 18:01 ` [PATCH] fix select() for SOCK_RAW sockets (ipv6) Stephen Hemminger @ 2004-12-07 18:48 ` David S. Miller 2004-12-07 18:56 ` Stephen Hemminger 2004-12-08 7:58 ` YOSHIFUJI Hideaki / 吉藤英明 0 siblings, 2 replies; 10+ messages in thread From: David S. Miller @ 2004-12-07 18:48 UTC (permalink / raw) To: Stephen Hemminger Cc: yoshfuji, mitch, kernel, linux-net, linux-kernel, netdev On Tue, 7 Dec 2004 10:01:40 -0800 Stephen Hemminger <shemminger@osdl.org> wrote: > > Probably, we need to do the same for ipv6, don't we? > > diff -Nru a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c > --- a/net/ipv6/af_inet6.c 2004-12-07 10:02:50 -08:00 > +++ b/net/ipv6/af_inet6.c 2004-12-07 10:02:50 -08:00 > @@ -513,6 +513,27 @@ We didn't do the "UDP select() fix" on ipv6 because we don't have the delayed checksumming optimization there. So the ipv6 side really doesn't need this change. Or did I miss something? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] fix select() for SOCK_RAW sockets (ipv6) 2004-12-07 18:48 ` David S. Miller @ 2004-12-07 18:56 ` Stephen Hemminger 2004-12-08 7:58 ` YOSHIFUJI Hideaki / 吉藤英明 1 sibling, 0 replies; 10+ messages in thread From: Stephen Hemminger @ 2004-12-07 18:56 UTC (permalink / raw) To: David S. Miller; +Cc: yoshfuji, mitch, kernel, linux-net, linux-kernel, netdev On Tue, 7 Dec 2004 10:48:15 -0800 "David S. Miller" <davem@davemloft.net> wrote: > On Tue, 7 Dec 2004 10:01:40 -0800 > Stephen Hemminger <shemminger@osdl.org> wrote: > > > > Probably, we need to do the same for ipv6, don't we? > > > > diff -Nru a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c > > --- a/net/ipv6/af_inet6.c 2004-12-07 10:02:50 -08:00 > > +++ b/net/ipv6/af_inet6.c 2004-12-07 10:02:50 -08:00 > > @@ -513,6 +513,27 @@ > > We didn't do the "UDP select() fix" on ipv6 because we don't > have the delayed checksumming optimization there. So the > ipv6 side really doesn't need this change. > > Or did I miss something? yeah, didn't look that deeply. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] fix select() for SOCK_RAW sockets (ipv6) 2004-12-07 18:48 ` David S. Miller 2004-12-07 18:56 ` Stephen Hemminger @ 2004-12-08 7:58 ` YOSHIFUJI Hideaki / 吉藤英明 1 sibling, 0 replies; 10+ messages in thread From: YOSHIFUJI Hideaki / 吉藤英明 @ 2004-12-08 7:58 UTC (permalink / raw) To: davem; +Cc: shemminger, mitch, kernel, linux-net, linux-kernel, netdev, yoshfuji In article <20041207104815.3f7a4684.davem@davemloft.net> (at Tue, 7 Dec 2004 10:48:15 -0800), "David S. Miller" <davem@davemloft.net> says: > On Tue, 7 Dec 2004 10:01:40 -0800 > Stephen Hemminger <shemminger@osdl.org> wrote: > > > > Probably, we need to do the same for ipv6, don't we? > > > > diff -Nru a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c > > --- a/net/ipv6/af_inet6.c 2004-12-07 10:02:50 -08:00 > > +++ b/net/ipv6/af_inet6.c 2004-12-07 10:02:50 -08:00 > > @@ -513,6 +513,27 @@ > > We didn't do the "UDP select() fix" on ipv6 because we don't > have the delayed checksumming optimization there. So the > ipv6 side really doesn't need this change. Ok, thanks for the explanation. --yoshfuji @ Strasbourg ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] fix select() for SOCK_RAW sockets 2004-12-07 15:08 ` [PATCH] fix select() for SOCK_RAW sockets Mitchell Blank Jr 2004-12-07 17:28 ` Phil Oester 2004-12-07 17:35 ` YOSHIFUJI Hideaki / 吉藤英明 @ 2004-12-07 17:45 ` Stephen Hemminger 2004-12-08 5:28 ` David S. Miller 3 siblings, 0 replies; 10+ messages in thread From: Stephen Hemminger @ 2004-12-07 17:45 UTC (permalink / raw) To: Mitchell Blank Jr Cc: Phil Oester, David S. Miller, linux-net, linux-kernel, netdev On Tue, 7 Dec 2004 07:08:34 -0800 Mitchell Blank Jr <mitch@sfgoth.com> wrote: > Phil: Here's a real patch for you to test. I actually left inet_dgram_ops > alone since it's an exported symbol (two of the users just want the .do_ioctl > value which is the same between SOCK_DGRAM and SOCK_RAW; the other is ipv6 > where it's clearly dealing with a UDP socket -- therefore I think its safest > to leave inet_dgram_ops to have the UDP behavior) > > Davem: I only tested that this doesn't break UDP; if it works for Phil and > Stephen can verify that it doesn't break his bad-checksum UDP tests then > please push it for 2.6.10. > Thanks, I'll retest UDP today, but it looks right. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] fix select() for SOCK_RAW sockets 2004-12-07 15:08 ` [PATCH] fix select() for SOCK_RAW sockets Mitchell Blank Jr ` (2 preceding siblings ...) 2004-12-07 17:45 ` [PATCH] fix select() for SOCK_RAW sockets Stephen Hemminger @ 2004-12-08 5:28 ` David S. Miller 3 siblings, 0 replies; 10+ messages in thread From: David S. Miller @ 2004-12-08 5:28 UTC (permalink / raw) To: Mitchell Blank Jr; +Cc: kernel, shemminger, linux-net, linux-kernel, netdev On Tue, 7 Dec 2004 07:08:34 -0800 Mitchell Blank Jr <mitch@sfgoth.com> wrote: > Davem: I only tested that this doesn't break UDP; if it works for Phil and > Stephen can verify that it doesn't break his bad-checksum UDP tests then > please push it for 2.6.10. Looks good Mitchell, patch applied. Thanks. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2004-12-08 7:58 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20041207003525.GA22933@linuxace.com>
[not found] ` <20041207025218.GB61527@gaz.sfgoth.com>
[not found] ` <20041207045302.GA23746@linuxace.com>
2004-12-07 5:48 ` Recent select() handling change breaks Poptop Mitchell Blank Jr
2004-12-07 15:08 ` [PATCH] fix select() for SOCK_RAW sockets Mitchell Blank Jr
2004-12-07 17:28 ` Phil Oester
2004-12-07 17:35 ` YOSHIFUJI Hideaki / 吉藤英明
2004-12-07 18:01 ` [PATCH] fix select() for SOCK_RAW sockets (ipv6) Stephen Hemminger
2004-12-07 18:48 ` David S. Miller
2004-12-07 18:56 ` Stephen Hemminger
2004-12-08 7:58 ` YOSHIFUJI Hideaki / 吉藤英明
2004-12-07 17:45 ` [PATCH] fix select() for SOCK_RAW sockets Stephen Hemminger
2004-12-08 5:28 ` David S. Miller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).