Messages in this thread | | | Date | Tue, 7 Nov 2006 22:14:32 +0100 (MET) | From | Jan Engelhardt <> | Subject | Re: write data to a file from a kernel module |
| |
> Hello, > > I am trying to create a module for kernel 2.4.27 that writes data to a > file when it > receives a packet from the network interface card. In the code below
What makes tcpdump and friends not usable that you need to write kernel code?
> the pkt_handler function is called each time a packet is received. A > while after the insmod, the CPU reaches 100% and the pc freezes. > > Is there something wrong with the code or is there an alternate way to > write data to a file? I think the problem is because the opening and > writing are happening too often. Don't tell me to use the proc > filesystem, it is > not appropriate in my case.
If at all, you should use a character device and do the copying with copy_to_user() rather than trying to open/write files in kernelspace.
> > Thanks in advance. > > (I've commented out the semaphore operations because they don't seem to > work as they are currently used) > > /* Packet handler. It is called by the kernel when a new packet has > been received. */ > int pkt_handler (struct sk_buff *skb, struct net_device *dv, struct > packet_type *pt) > { > static int calls = 0;
Your indentation is broekn, it's hard to read.
> kfree_skb (skb); > > printk (KERN_ALERT "packet %d received\n", calls); > if (calls++ > 10) { > open_f_and_write ("/tmp/tmp.log");
You repeatedly open and write. That's a CPU buster for sure. How about opening it once? (Of course that leaves the question where to close it - which is why you should use alternate methods such as char devs, (proc) or SOCK_RAW)
open_f_and_write()s return value is not examined. In case you run into a problem, you will continue to do so -- not so good, better stop when there is a problem.
> calls = 0; > } > > return 0; > } > > > int open_f_and_write (const char *file_name) > { > struct file *file = NULL; > mm_segment_t fs; > char *tmp; > struct inode *inode; > > tmp = getname (file_name); > > file = filp_open (tmp, O_CREAT | O_WRONLY | O_APPEND, S_IRWXU); > if (IS_ERR (file)) {
tmp is not freed. So it's more than just logical for the box to lockup someday.
> int errno = PTR_ERR (file); > printk (KERN_DEBUG "error %i\n", errno); > return 2;
You usually would return -PTR_ERR(file) here;
> } > if (!file->f_op->write) { > fput (file);
tmp is not freed here either.
> return 3; > } > > fs = get_fs (); > set_fs (KERNEL_DS); > //inode = file->f_dentry->d_inode; > //down (&inode->i_sem); > > { > char *buffer = "write something\n"; > int p = file->f_op->write (file, buffer, m_strlen (buffer),
strlen(buffer) is probably better, whatever m_strlen() is.
> &file->f_pos); > printk (KERN_ALERT "write returned %d\n", p); > } > //up(&inode->i_sem); > set_fs (fs); > putname (tmp); > filp_close (file, NULL); > > return 0; > } > - > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > >
-`J' -- - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |