From: Radek Matejka Date: Tue, 14 Aug 2012 15:04:57 +0000 (+0200) Subject: 2xCEGW_LISTEN race condition protection, clean exit X-Git-Url: http://rtime.felk.cvut.cz/gitweb/can-eth-gw.git/commitdiff_plain/4d416bf074ee37fe79aaac3986d92d322766d1fe 2xCEGW_LISTEN race condition protection, clean exit --- diff --git a/kernel/canethgw.c b/kernel/canethgw.c index 28f8c2a..7686039 100644 --- a/kernel/canethgw.c +++ b/kernel/canethgw.c @@ -18,34 +18,21 @@ #include #include -/** - * ToDo: - * [ ] encapsule module - check inputs - * [ ] refactor - chc .h - * [ ] dump callback - * [ ] rtnl vs nl functions - * [ ] stop threads - * [ ] change listening - * [ ] clean exit - threads, jobs - */ - MODULE_LICENSE( "GPL" ); static int cegw_udp_can( void* data ); inline static void cegw_udp_send( struct socket* udp_sock, struct can_frame* cf, struct in_addr ipaddr, u16 port ); static int cegw_can_udp( void* data ); inline static void cegw_can_send( struct socket* can_sock, struct can_frame* cf, int ifindex ); -static int cegw_thread_start( void ); -static void cegw_thread_stop( void ); -static int cegw_thread_restart( void* arg ); +static int cegw_thread_start( void* data ); +static int cegw_thread_stop( void ); -#define CEGW_STOPPED 0 -#define CEGW_RUNNING 1 - -static struct socket* can_sock = NULL, * udp_sock = NULL; -static struct task_struct* eth_to_can = NULL, * can_to_eth = NULL; -/* ToDo: protect with mutex */ -static int cegw_state = CEGW_STOPPED; +enum __cegw_state +{ + CEGW_RUN, + CEGW_STOP, + CEGW_EXIT +}; struct cegw_rule { @@ -55,17 +42,19 @@ struct cegw_rule struct hlist_node list; }; -HLIST_HEAD( cegw_rule_can_eth ); -HLIST_HEAD( cegw_rule_eth_can ); - -struct +struct cegw_setting { - int can_ifindex; - struct in_addr eth_addr; + struct in_addr eth_ip; unsigned short eth_port; -} cegw_setting; +}; + +static int cegw_state = CEGW_STOP; +static struct socket* can_sock = NULL, * udp_sock = NULL; +static struct task_struct* eth_to_can = NULL, * can_to_eth = NULL; -DEFINE_MUTEX( cegw_setting_mutex ); +HLIST_HEAD( cegw_rule_can_eth ); +HLIST_HEAD( cegw_rule_eth_can ); +DEFINE_MUTEX( cegw_mutex ); inline static void cegw_udp_send( struct socket* udp_sock, struct can_frame* cf, struct in_addr ipaddr, u16 port ) { @@ -134,7 +123,7 @@ static int cegw_udp_can( void* data ) while( 1 ) { - if( cegw_state == CEGW_STOPPED ) + if( cegw_state == CEGW_STOP ) break; vec.iov_base = &cf; vec.iov_len = sizeof(cf); @@ -167,7 +156,7 @@ static int cegw_can_udp( void* data ) struct sockaddr_can ca; struct cegw_rule* rule; struct hlist_node* pos; - struct in_addr eth_addr; + struct in_addr eth_ip; u16 eth_port; int recv_size; @@ -179,7 +168,7 @@ static int cegw_can_udp( void* data ) while( 1 ) { - if( cegw_state == CEGW_STOPPED ) /**/ + if( cegw_state == CEGW_STOP ) /**/ break; vec.iov_base = &cf; vec.iov_len = sizeof( cf ); @@ -192,11 +181,11 @@ static int cegw_can_udp( void* data ) hlist_for_each_entry_rcu( rule, pos, &cegw_rule_can_eth, list ) { rcu_read_lock(); - eth_addr = rule->eth_ip; + eth_ip = rule->eth_ip; eth_port = rule->eth_port; rcu_read_unlock(); if( rule->can_ifindex == ca.can_ifindex ) - cegw_udp_send( udp_sock, &cf, eth_addr, eth_port ); + cegw_udp_send( udp_sock, &cf, eth_ip, eth_port ); } } @@ -213,6 +202,7 @@ static int cegw_create_job( struct sk_buff* skb, struct nlmsghdr* nlh, void* arg struct rtmsg* r; struct in_addr ip; unsigned short port; + struct cegw_setting* set; int err = 0; if( nlmsg_len(nlh) < sizeof(*r) ) @@ -245,11 +235,11 @@ static int cegw_create_job( struct sk_buff* skb, struct nlmsghdr* nlh, void* arg return -EINVAL; } - mutex_lock( &cegw_setting_mutex ); - cegw_setting.eth_addr = *(struct in_addr*)nla_data( tb[CEGW_ETH_IP] ); - cegw_setting.eth_port = *(unsigned short*)nla_data( tb[CEGW_ETH_PORT] ); - kthread_run( cegw_thread_restart, NULL, "canethgw" ); - mutex_unlock( &cegw_setting_mutex ); + /* ToDo: valid listen address */ + set = kmalloc( sizeof(*set), GFP_KERNEL ); + set->eth_ip = *(struct in_addr*)nla_data( tb[CEGW_ETH_IP] ); + set->eth_port = *(unsigned short*)nla_data( tb[CEGW_ETH_PORT] ); + kthread_run( cegw_thread_start, set, "canethgw" ); break; case CEGW_RULE_CAN_ETH: if( !tb[CEGW_ETH_IP] || !tb[CEGW_ETH_PORT] || !tb[CEGW_CAN_IFINDEX] ) @@ -307,13 +297,27 @@ static int cegw_create_job( struct sk_buff* skb, struct nlmsghdr* nlh, void* arg return 0; } -static int cegw_remove_job( struct sk_buff* skb, struct nlmsghdr* nlh, void* arg ) +static void cegw_flush( void ) { - struct rtmsg* r; - struct nlattr* tb[ CEGW_MAX+1 ]; struct hlist_node* pos,* n; struct cegw_rule* rule; + hlist_for_each_entry_safe( rule, pos, n, &cegw_rule_can_eth, list ) + { + hlist_del( &rule->list ); + kfree( rule ); + } + hlist_for_each_entry_safe( rule, pos, n, &cegw_rule_eth_can, list ) + { + hlist_del( &rule->list ); + kfree( rule ); + } +} + +static int cegw_remove_job( struct sk_buff* skb, struct nlmsghdr* nlh, void* arg ) +{ + struct rtmsg* r; + struct nlattr* tb[ CEGW_MAX+1 ]; int err = 0; if( nlmsg_len(nlh) < sizeof(*r) ) @@ -339,17 +343,8 @@ static int cegw_remove_job( struct sk_buff* skb, struct nlmsghdr* nlh, void* arg return -EINVAL; } - hlist_for_each_entry_safe( rule, pos, n, &cegw_rule_can_eth, list ) - { - hlist_del( &rule->list ); - kfree( rule ); - } - hlist_for_each_entry_safe( rule, pos, n, &cegw_rule_eth_can, list ) - { - hlist_del( &rule->list ); - kfree( rule ); - } - + cegw_flush(); + return 0; } @@ -441,17 +436,34 @@ brk: return skb->len; } -static int cegw_thread_start( void ) +/** + * cegw_thread_start - start working threads + * Two threads are started. One is serving udp->can routing and the other + * can->udp. + * + * @return 0 on success, -1 otherwise + */ +static int cegw_thread_start( void* data ) { struct sockaddr_in udp_addr; struct sockaddr_can can_addr; + struct cegw_setting* set; + + set = (struct cegw_setting*)data; can_addr.can_family = AF_CAN; can_addr.can_ifindex = 0; udp_addr.sin_family = AF_INET; - udp_addr.sin_port = htons( cegw_setting.eth_port ); - udp_addr.sin_addr = cegw_setting.eth_addr; + udp_addr.sin_port = htons( set->eth_port ); + udp_addr.sin_addr = set->eth_ip; + + kfree( set ); + mutex_lock( &cegw_mutex ); + if( cegw_state == CEGW_EXIT ) + return -1; + /* stops threads if exist */ + cegw_thread_stop(); /* create and bind sockets */ if( sock_create_kern( PF_INET, SOCK_DGRAM, IPPROTO_UDP, &udp_sock) != 0 ) @@ -471,49 +483,66 @@ static int cegw_thread_start( void ) printk( KERN_ERR "canethgw: udp socket binding failed\n" ); sock_release( udp_sock ); sock_release( can_sock ); - return -1; /* ToDo: state==RUNNING and return? */ + return -1; } if( kernel_bind( can_sock, (struct sockaddr*) &can_addr, sizeof(can_addr) ) != 0 ) { printk( KERN_ERR "canethgw: can socket binding failed\n" ); + kernel_sock_shutdown( udp_sock, SHUT_RDWR ); sock_release( udp_sock ); sock_release( can_sock ); return -1; } /* start threads */ - cegw_state = CEGW_RUNNING; + cegw_state = CEGW_RUN; + eth_to_can = kthread_create( cegw_udp_can, NULL, "canethgw" ); - if( !IS_ERR( eth_to_can ) ) - { - get_task_struct( eth_to_can ); - wake_up_process( eth_to_can ); - } + if( IS_ERR( eth_to_can ) ) + { + cegw_state = CEGW_STOP; + sock_release( udp_sock ); + sock_release( can_sock ); + return -1; + } + get_task_struct( eth_to_can ); + wake_up_process( eth_to_can ); + can_to_eth = kthread_create( cegw_can_udp, NULL, "canethgw" ); - if( !IS_ERR( can_to_eth ) ) + if( IS_ERR( can_to_eth ) ) { - /* ToDo: free this? */ - get_task_struct( can_to_eth ); - wake_up_process( can_to_eth ); + cegw_state = CEGW_STOP; + kernel_sock_shutdown( udp_sock, SHUT_RDWR ); + kthread_stop( eth_to_can ); + sock_release( udp_sock ); + sock_release( can_sock ); + return -1; } + /* ToDo: free this? */ + get_task_struct( can_to_eth ); + wake_up_process( can_to_eth ); - /* ToDo: kthread creation fail */ + mutex_unlock( &cegw_mutex ); pr_devel( "threads are running\n" ); - return 0; } -static void cegw_thread_stop( void ) +/** + * cegw_thread_stop + * Waits for threads to stop. Does nothing if cegw_state == CEGW_STOP. + * + * @return 0 + */ +static int cegw_thread_stop( void ) { int how = SHUT_RDWR; struct sock* sk = NULL; - /* no threads are running */ - if( !can_to_eth && !eth_to_can ) - return; - - cegw_state = CEGW_STOPPED; + if( cegw_state == CEGW_STOP ) + return 0; + + cegw_state = CEGW_STOP; /* shut down socket */ sk = can_sock->sk; how++; @@ -531,23 +560,10 @@ static void cegw_thread_stop( void ) sock_release( can_sock ); can_to_eth = NULL; eth_to_can = NULL; -} - -/** - * cegw_thread_restart - */ -static int cegw_thread_restart( void* data ) -{ - cegw_thread_stop(); - cegw_thread_start(); return 0; } -/*********************** - * module init/exit - ***********************/ - static int __init cegw_init( void ) { /* subscribe to netlink */ @@ -560,15 +576,19 @@ static int __init cegw_init( void ) static void __exit cegw_exit( void ) { - /* ToDo: effect on others? */ + /* ToDo: effect on cangw? */ rtnl_unregister_all( PF_CAN ); - /* ToDo: no rtnl pending? */ + /* wait for rtnl callbacks */ + rtnl_lock(); + rtnl_unlock(); + + mutex_lock( &cegw_mutex ); cegw_thread_stop(); - /* ToDo: frees mem_cache? */ - /* udp must not exists */ + cegw_state = CEGW_EXIT; + mutex_unlock( &cegw_mutex ); - printk( "cangw: exit\n" ); + cegw_flush(); } module_init( cegw_init );