Re: [PATCH] CONFIG_PACKET_MMAP should depend on MMU
On 4/20/07, David Howells <[EMAIL PROTECTED]> wrote: Aubrey Li <[EMAIL PROTECTED]> wrote: > as checked in packet_set_ring, buffer size must be a multiple of PAGE_SIZE, > packet_set_ring > if (unlikely(req->tp_block_size & (PAGE_SIZE - 1))) > > So why not use __get_free_pages rather than kmalloc, Because kmalloc() may be able to get us a smaller chunk of memory. Actually, calling __get_free_pages() might be a better, and then release the excess pages. so that we have pagetables to count? There are no pagetables in NOMMU-mode. Hmm..., I'm thinking some codes in memory reclaim depend on NR_FILE_MMAPED like follows snip if (zone_page_state(zone, NR_FILE_PAGES) - zone_page_state(zone, NR_FILE_MAPPED) > zone->min_unmapped_pages) snip Since we are enabling packet mmap feature, we should take its mapped pages into count of NR_FILE_MAPPED. -Aubrey - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] CONFIG_PACKET_MMAP should depend on MMU
On 4/20/07, David Howells <[EMAIL PROTECTED]> wrote: Aubrey Li <[EMAIL PROTECTED]> wrote: > The patch works properly on my side. But > 1) I'm not sure why you re-wrote alloc/free_pg_vec function, doesn't > the current implement work for NOMMU? I know you want to allocate the > entire data buffer as one contiguous lump, but is it really necessary? Yes. It's not possible to map the whole buffer otherwise. Think about it! mmap() returns _one_ reference address. In MMU-mode, the non-contiguous physical buffers can be made to appear virtually contiguous by fudging the page tables and using the MMU. This is not possible in NOMMU-mode. The app will expect the buffer to be one contiguous lump in its address space, and will not be able to locate the other segments of the buffer. Great explanation, thanks, :-) Actually, what I said is not quite true. It is possible to map the whole buffer otherwise: I could lift the restriction that requires that you map the whole buffer or not at all, and then userspace could stitch the whole lot together itself. This would then require userspace to be bimodal. > 2) So the mapped pages doesn't count into NR_FILE_MAPPED, is it a problem? Not really, no - there are no pagetables. Furthermore, issuing the PACKET_RX_RING sockopt does the entire allocation. Any subsequent mmaps on it have little effect. We could do that accounting though if you think it'd be better. I don't suppose it hurts. as checked in packet_set_ring, buffer size must be a multiple of PAGE_SIZE, packet_set_ring if (unlikely(req->tp_block_size & (PAGE_SIZE - 1))) So why not use __get_free_pages rather than kmalloc, so that we have pagetables to count? -Aubrey - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] CONFIG_PACKET_MMAP should depend on MMU
On 4/18/07, David Howells <[EMAIL PROTECTED]> wrote:
Aubrey Li <[EMAIL PROTECTED]> wrote:
> Here, in the attachment I wrote a small test app. Please correct if
> there is anything wrong, and feel free to improve it.
Okay... I have that working... probably. I don't know what output it's
supposed to produce, but I see this:
# /packet-mmap/sample_packet_mmap
00-00-00-01-00-00-00-8a-00-00-00-8a-00-42-00-50-
38-43-13-a0-00-07-ff-3c-00-00-00-00-00-00-00-00-
00-11-08-00-00-00-00-01-00-01-00-06-00-d0-b7-de-
32-7b-00-00-00-00-00-00-00-00-00-00-00-00-00-00-
00-00-00-90-cc-a2-75-6b-00-d0-b7-de-32-7b-08-00-
45-00-00-7c-00-00-40-00-40-11-b4-13-c0-a8-02-80-
c0-a8-02-8d-08-01-03-20-00-68-8e-65-7f-5b-7e-03-
00-00-00-01-00-00-00-00-00-00-00-00-00-00-00-00-
00-00-00-00-00-00-00-00-00-00-00-01-00-00-81-a4-
00-00-00-01-00-00-00-00-00-00-00-00-00-1d-b8-86-
00-00-10-00-ff-ff-ff-ff-00-00-0e-f0-00-00-09-02-
01-cb-03-16-46-26-38-0d-00-00-00-00-46-26-38-1e-
00-00-00-00-46-26-38-1e-00-00-00-00-00-00-00-00-
00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00- [repeated]
Does that look reasonable?
I've attached the preliminary patch. Note four things about it:
(1) I've had to add the get_unmapped_area() op to the proto_ops struct, but
I've only done it for CONFIG_MMU=n as making it available for CONFIG_MMU=y
could cause problems.
(2) There's a race between packet_get_unmapped_area() being called and
packet_mmap() being called.
(3) I've added an extra check into packet_set_ring() to make sure the caller
isn't asking for a combination of buffer size and count that will exceed
ULONG_MAX. This protects a multiply done elsewhere.
(4) The entire data buffer is allocated as one contiguous lump in NOMMU-mode.
David
---
[PATCH] NOMMU: Support mmap() on AF_PACKET sockets
From: David Howells <[EMAIL PROTECTED]>
Support mmap() on AF_PACKET sockets in NOMMU-mode kernels.
Signed-Off-By: David Howells <[EMAIL PROTECTED]>
---
include/linux/net.h|7 +++
include/net/sock.h |8 +++
net/core/sock.c| 10
net/packet/af_packet.c | 118
net/socket.c | 77 +++
5 files changed, 219 insertions(+), 1 deletions(-)
diff --git a/include/linux/net.h b/include/linux/net.h
index 4db21e6..9e77cf6 100644
--- a/include/linux/net.h
+++ b/include/linux/net.h
@@ -161,6 +161,11 @@ struct proto_ops {
int (*recvmsg) (struct kiocb *iocb, struct socket *sock,
struct msghdr *m, size_t total_len,
int flags);
+#ifndef CONFIG_MMU
+ unsigned long (*get_unmapped_area)(struct file *file, struct socket
*sock,
+unsigned long addr, unsigned long
len,
+unsigned long pgoff, unsigned long
flags);
+#endif
int (*mmap) (struct file *file, struct socket *sock,
struct vm_area_struct * vma);
ssize_t (*sendpage) (struct socket *sock, struct page *page,
@@ -191,6 +196,8 @@ extern int sock_sendmsg(struct socket *sock,
struct msghdr *msg,
extern int sock_recvmsg(struct socket *sock, struct msghdr *msg,
size_t size, int flags);
extern int sock_map_fd(struct socket *sock);
+extern void sock_make_mappable(struct socket *sock,
+ unsigned long prot);
extern struct socket *sockfd_lookup(int fd, int *err);
#define sockfd_put(sock) fput(sock->file)
extern int net_ratelimit(void);
diff --git a/include/net/sock.h b/include/net/sock.h
index 2c7d60c..d91edea 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -841,6 +841,14 @@ extern int sock_no_sendmsg(struct
kiocb *, struct socket *,
struct msghdr *, size_t);
extern int sock_no_recvmsg(struct kiocb *, struct socket
*,
struct msghdr *, size_t, int);
+#ifndef CONFIG_MMU
+extern unsigned long sock_no_get_unmapped_area(struct file *,
+ struct socket *,
+ unsigned long,
+ unsigned long,
+ unsigned long,
+ unsigned long);
+#endif
extern int sock_no_mmap(struct file *file,
struct socket *sock,
Re: [PATCH] CONFIG_PACKET_MMAP should depend on MMU
On 4/18/07, David Howells <[EMAIL PROTECTED]> wrote:
Aubrey Li <[EMAIL PROTECTED]> wrote:
> Here, in the attachment I wrote a small test app. Please correct if
> there is anything wrong, and feel free to improve it.
Okay... I have that working... probably. I don't know what output it's
supposed to produce, but I see this:
# /packet-mmap/sample_packet_mmap
00-00-00-01-00-00-00-8a-00-00-00-8a-00-42-00-50-
38-43-13-a0-00-07-ff-3c-00-00-00-00-00-00-00-00-
00-11-08-00-00-00-00-01-00-01-00-06-00-d0-b7-de-
32-7b-00-00-00-00-00-00-00-00-00-00-00-00-00-00-
00-00-00-90-cc-a2-75-6b-00-d0-b7-de-32-7b-08-00-
45-00-00-7c-00-00-40-00-40-11-b4-13-c0-a8-02-80-
c0-a8-02-8d-08-01-03-20-00-68-8e-65-7f-5b-7e-03-
00-00-00-01-00-00-00-00-00-00-00-00-00-00-00-00-
00-00-00-00-00-00-00-00-00-00-00-01-00-00-81-a4-
00-00-00-01-00-00-00-00-00-00-00-00-00-1d-b8-86-
00-00-10-00-ff-ff-ff-ff-00-00-0e-f0-00-00-09-02-
01-cb-03-16-46-26-38-0d-00-00-00-00-46-26-38-1e-
00-00-00-00-46-26-38-1e-00-00-00-00-00-00-00-00-
00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00- [repeated]
Does that look reasonable?
Yes, it's reasonable for me, as long as your
host IP is 192.168.2.128
and
target IP is 192.168.2.141
See below
00-90-cc-a2-75-6b-|___ MAC Address
00-d0-b7-de-32-7b-|
08-00Type: IP
45-00Ver, IHL, TOS
00-7cIP.total.length
00-00-
40-00-
40TTL
11UDP protocol
b4-13Checksum
c0-a8-02-80---Source IP: 192.168.2.128
c0-a8-02-8d---Dest IP: 192.168.2.141
snip--
I've attached the preliminary patch.
Thanks, I'll take a look and try to see if I can give some feedback.
-Aubrey
Note four things about it:
(1) I've had to add the get_unmapped_area() op to the proto_ops struct, but
I've only done it for CONFIG_MMU=n as making it available for CONFIG_MMU=y
could cause problems.
(2) There's a race between packet_get_unmapped_area() being called and
packet_mmap() being called.
(3) I've added an extra check into packet_set_ring() to make sure the caller
isn't asking for a combination of buffer size and count that will exceed
ULONG_MAX. This protects a multiply done elsewhere.
(4) The entire data buffer is allocated as one contiguous lump in NOMMU-mode.
David
---
[PATCH] NOMMU: Support mmap() on AF_PACKET sockets
From: David Howells <[EMAIL PROTECTED]>
Support mmap() on AF_PACKET sockets in NOMMU-mode kernels.
Signed-Off-By: David Howells <[EMAIL PROTECTED]>
---
include/linux/net.h|7 +++
include/net/sock.h |8 +++
net/core/sock.c| 10
net/packet/af_packet.c | 118
net/socket.c | 77 +++
5 files changed, 219 insertions(+), 1 deletions(-)
diff --git a/include/linux/net.h b/include/linux/net.h
index 4db21e6..9e77cf6 100644
--- a/include/linux/net.h
+++ b/include/linux/net.h
@@ -161,6 +161,11 @@ struct proto_ops {
int (*recvmsg) (struct kiocb *iocb, struct socket *sock,
struct msghdr *m, size_t total_len,
int flags);
+#ifndef CONFIG_MMU
+ unsigned long (*get_unmapped_area)(struct file *file, struct socket
*sock,
+unsigned long addr, unsigned long
len,
+unsigned long pgoff, unsigned long
flags);
+#endif
int (*mmap) (struct file *file, struct socket *sock,
struct vm_area_struct * vma);
ssize_t (*sendpage) (struct socket *sock, struct page *page,
@@ -191,6 +196,8 @@ extern int sock_sendmsg(struct socket *sock,
struct msghdr *msg,
extern int sock_recvmsg(struct socket *sock, struct msghdr *msg,
size_t size, int flags);
extern int sock_map_fd(struct socket *sock);
+extern void sock_make_mappable(struct socket *sock,
+ unsigned long prot);
extern struct socket *sockfd_lookup(int fd, int *err);
#define sockfd_put(sock) fput(sock->file)
extern int net_ratelimit(void);
diff --git a/include/net/sock.h b/include/net/sock.h
index 2c7d60c..d91edea 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -841,6 +841,14 @@ extern int sock_no_sendmsg(struct
kiocb *, struct socket *,
struct msghdr *, size
Re: [PATCH] CONFIG_PACKET_MMAP should depend on MMU
On 4/11/07, Robin Getz <[EMAIL PROTECTED]> wrote: On Tue 10 Apr 2007 08:55, David Howells pondered: > Looking at alloc_pg_vec() in af_packet.c, I will place my bets on the > latter case. I don't know that this is a problem; it depends on how things > work, and that I don't know offhand. If someone can give me a simple test > program, I would be able to evaluate it better. Hmm - the only think I have used in the past is tcpdump/libpcap from http://public.lanl.gov/cpw/ Documentation/networking/packet_mmap.txt also seems to be a little dated, but does have some code snippets if you wanted to make something lightweight... Does anyone else on netdev have a small test app? Here, in the attachment I wrote a small test app. Please correct if there is anything wrong, and feel free to improve it. -Aubrey packet-mmap.tar.gz Description: GNU Zip compressed data
[PATCH] Fix UDP checksum issue in net poll mode.
In net poll mode, the current checksum function doesn't consider the kind of packet which is padded to reach a specific minimum length. I believe that's the problem causing my test case failed. The following patch fixed this issue. Signed-off-by: Aubrey.Li <[EMAIL PROTECTED]> --- net/core/netpoll.c |7 +++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/net/core/netpoll.c b/net/core/netpoll.c index 823215d..522e441 100644 --- a/net/core/netpoll.c +++ b/net/core/netpoll.c @@ -471,6 +471,13 @@ int __netpoll_rx(struct sk_buff *skb) if (skb->len < len || len < iph->ihl*4) goto out; + /* +* Our transport medium may have padded the buffer out. +* Now We trim to the true length of the frame. +*/ + if (pskb_trim_rcsum(skb, len)) + goto out; + if (iph->protocol != IPPROTO_UDP) goto out; -- 1.5.1 - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: udp checksum issue in netpoll mode.
On 4/12/07, Stephen Hemminger <[EMAIL PROTECTED]> wrote: Aubrey Li wrote: > I think we discussed this issue before. > > The current checksum function doesn't consider the kind of packet > which is padded to reach a specific minimum length. I believe that's > the problem caused my test case failed. Is this issue fixed? Or is it > acceptable if I make a patch not calculating this kind of packet? > > Thanks, > -Aubrey The caller should be trimming the packet and updating the hardware checksum value (like the normal UDP path). OK, the following patch fixed the problem. Signed-off-by: Aubrey.Li <[EMAIL PROTECTED]> --- net/core/netpoll.c |7 +++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/net/core/netpoll.c b/net/core/netpoll.c index 823215d..522e441 100644 --- a/net/core/netpoll.c +++ b/net/core/netpoll.c @@ -471,6 +471,13 @@ int __netpoll_rx(struct sk_buff *skb) if (skb->len < len || len < iph->ihl*4) goto out; + /* +* Our transport medium may have padded the buffer out. +* Now We trim to the true length of the frame. +*/ + if (pskb_trim_rcsum(skb, len)) + goto out; + if (iph->protocol != IPPROTO_UDP) goto out; -- 1.5.1 - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
udp checksum issue in netpoll mode.
I think we discussed this issue before.
The current checksum function doesn't consider the kind of packet
which is padded to reach a specific minimum length. I believe that's
the problem caused my test case failed. Is this issue fixed? Or is it
acceptable if I make a patch not calculating this kind of packet?
Thanks,
-Aubrey
===
static __sum16 checksum_udp(struct sk_buff *skb, struct udphdr *uh,
unsigned short ulen, __be32 saddr, __be32 daddr)
{
__wsum psum;
if (uh->check == 0 || skb->ip_summed == CHECKSUM_UNNECESSARY)
return 0;
psum = csum_tcpudp_nofold(saddr, daddr, ulen, IPPROTO_UDP, 0);
if (skb->ip_summed == CHECKSUM_COMPLETE &&
!csum_fold(csum_add(psum, skb->csum)))
return 0;
skb->csum = psum;
return __skb_checksum_complete(skb);
}
===
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] CONFIG_PACKET_MMAP should depend on MMU
The option CONFIG_PACKET_MMAP should depend on MMU. Signed-off-by: Aubrey.Li <[EMAIL PROTECTED]> --- net/packet/Kconfig |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/net/packet/Kconfig b/net/packet/Kconfig index 34ff93f..959c272 100644 --- a/net/packet/Kconfig +++ b/net/packet/Kconfig @@ -17,7 +17,7 @@ config PACKET config PACKET_MMAP bool "Packet socket: mmapped IO" - depends on PACKET + depends on PACKET && MMU help If you say Y here, the Packet protocol driver will use an IO mechanism that results in faster communication. -- 1.5.1 - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
