(usagi-users 03862) Re: Some UMIP fixes

Romain KUNTZ <[email protected]> Fri, 22 Jun 2007 14:49:44 +0200
Newsgroups gmane.linux.ipv6.usagi.users
Message-ID <[email protected]>
Hi Takamiya-san,

Thanks for taking the time to review the patch. I have splitted the  
patch into several small ones and enclosed them to this mail. some  
comments inline:

On 2007/06/21, at 15:45, Noriaki TAKAMIYA wrote:
>> * chkconf_kernel.sh
>> The chkconf_kernel.sh script is not up-to-date with the 2.6.21 kernel
>> options. The patch fixes that by fixing a typo for XFRM_SUB_POLICY
>> and removing the check for the IPV6_ADVANCED_ROUTER option.
>>
>> * config.h.in, configure.ac, mh.c
>> This is actually the fix sent few days ago by Masahide Nakamura in
>> the MIPL ML for the RH issue.
>
>   These two fixes are fine.

The patches are chkconf_kernel.sh.diff and rth2-nakamura-fix.diff


>> * src/Makefile.am
>> I am experiencing an issue on some PC config at compilation time when
>> the gram.y file is processed: gram.y includes mn.h and complains
>> about pthread_rwlock_t that is used in mn.h (whereas no other file
>> including mn.h ever complains).
>> Adding -D_GNU_SOURCE flag at compilation fixes the problem (although
>> this should be implicit with the AC_GNU_SOURCE located in the
>> confiure.ac file, this does not work with this specific gram.y file.
>> Maybe related to a lex/yacc issue?)
>
>   Could you show your environment?
>
>   We don't see such a problem on the following environment:
>
>   - Debian Etch
>   - SuSE 9.3
>   - Ubuntu 7.0.4 Feisty Fawn

It happened on a Debian Lenny with autoconf 2.61-4, flex 2.5.33-11,  
byacc 20050813-1
Could you provide me your autoconf, flex, byacc version? I'll try to  
dig this issue a bit more.

In case you'd need the patch, the fix is in makefile-gnu-source.diff

>
>> * src/mn.c
>> Cosmetic change (fixes a typo)
>
>   It looks fine.
>
>> * src/proc_sys.h
>> - Cosmetic change to avoid warnings at compilation time.
>
>   It is OK.

Both fixes are in cosmetics.diff

>> * src/movement.c
>> Several things here:
>> - Cosmetic changes to avoid warnings at compilation time.
>> - Changed the definition of process_nlmsg to avoid warnings at
>> compoilation
>> - There is an issue regarding some of the BLACKHOLE rules installed
>> by  md_block_rule_add/del and mn_block_rule_add/del: when exiting
>> mip6d, I always have a remaining "from all blackhole" rule.
>
>   It should be fine, but can I have more time to check?
>   I find the other problem, but it is another issue.
>
>> * src/rtnl.h
>> - Added a new priority for the HoA blackhole rule
>
>   It is related to the fix in src/movement.c, OK.

Sure, take time to fix your other problem.
Those fixes are in hoa-blackhole-fix.diff

>> * src/xfrm.c
>> - mip6d tries to install a policy that already exists and fails. Thus
>> we update it instead. See the comment in the patch (starting with  
>> RK).
>
>   It is OK.

The fix is in xfrm.c.diff (but you may want to remove the commented  
lines once the fix has been applied).

Regards,
-- 
Romain KUNTZ
[email protected]
Louis Pasteur University - Networks and Protocols Team
chkconf_kernel.sh.diff (application/octet-stream, 927 B)
diff -Nur mipv6-daemon-umip-0.3/chkconf_kernel.sh mipv6-daemon-umip-0.3-fix/chkconf_kernel.sh
--- mipv6-daemon-umip-0.3/chkconf_kernel.sh	2007-03-29 16:51:31.000000000 +0200
+++ mipv6-daemon-umip-0.3-fix/chkconf_kernel.sh	2007-06-13 16:05:55.000000000 +0200
@@ -24,19 +24,18 @@
 IPV6=y
 INET6_ESP=y
 IPV6_TUNNEL=y
-IPV6_ADVANCED_ROUTER=y
 IPV6_MULTIPLE_TABLES=y
 IPV6_SUBTREES=y
 IPV6_MIP6=y
 XFRM=y
 XFRM_USER=y
-CONFIG_XFRM_SUB_POLICY=y
+XFRM_SUB_POLICY=y
 INET6_XFRM_MODE_ROUTEOPTIMIZATION=y
 
 COMMON="EXPERIMENTAL SYSVIPC PROC_FS NET INET IPV6 IPV6_MIP6 \
 	XFRM XFRM_USER XFRM_SUB_POLICY INET6_XFRM_MODE_ROUTEOPTIMIZATION"
 
-MNHA="IPV6_TUNNEL IPV6_ADVANCED_ROUTER IPV6_MULTIPLE_TABLES"
+MNHA="IPV6_TUNNEL IPV6_MULTIPLE_TABLES"
 
 MN="IPV6_SUBTREES"
 
@@ -46,7 +45,7 @@
 
 TAGS="$COMMON $MNHA $MN $IPSEC $PFKEY"
 
-if [ "$1" == "" ] ; then
+if [ "$1" = "" ] ; then
     KERNELCONFIG="/proc/config.gz"
     CAT=zcat
 else
cosmetics.diff (application/octet-stream, 1.3 KB)
diff -Nur mipv6-daemon-umip-0.3/src/mn.c mipv6-daemon-umip-0.3-fix/src/mn.c
--- mipv6-daemon-umip-0.3/src/mn.c	2007-03-29 16:51:31.000000000 +0200
+++ mipv6-daemon-umip-0.3-fix/src/mn.c	2007-06-13 16:05:55.000000000 +0200
@@ -129,7 +129,7 @@
 	if ((ret = rule_add(NULL, 0, IP6_RULE_PRIO_MIP6_BLOCK, RTN_BLACKHOLE,
 		     &hai->hoa.addr, 128, &in6addr_any, 0,
 		     FIB_RULE_FIND_SADDR)) < 0)
-		MDBG("failed to delete blackhole rule.\n");
+		MDBG("failed to add blackhole rule.\n");
 	else
 		hai->home_block |= HOME_ADDR_RULE_BLOCK;
 
diff -Nur mipv6-daemon-umip-0.3/src/proc_sys.h mipv6-daemon-umip-0.3-fix/src/proc_sys.h
--- mipv6-daemon-umip-0.3/src/proc_sys.h	2007-03-29 16:51:31.000000000 +0200
+++ mipv6-daemon-umip-0.3-fix/src/proc_sys.h	2007-06-13 16:05:55.000000000 +0200
@@ -4,7 +4,7 @@
 #define __PROC_SYS_H__ 1
 
 #define PROC_SYS_IP6_AUTOCONF "/proc/sys/net/ipv6/conf/%s/autoconf"
-#define PROC_SYS_IP6_ACCEPT_RA"/proc/sys/net/ipv6/conf/%s/accept_ra"
+#define PROC_SYS_IP6_ACCEPT_RA "/proc/sys/net/ipv6/conf/%s/accept_ra"
 #define PROC_SYS_IP6_ACCEPT_RA_DEFRTR "/proc/sys/net/ipv6/conf/%s/accept_ra_defrtr"
 #define PROC_SYS_IP6_ACCEPT_RA_PINFO "/proc/sys/net/ipv6/conf/%s/accept_ra_pinfo"
 #define PROC_SYS_IP6_RTR_SOLICITS "/proc/sys/net/ipv6/conf/%s/router_solicitations"
hoa-blackhole-fix.diff (application/octet-stream, 2.2 KB)
diff -Nur mipv6-daemon-umip-0.3/src/movement.c mipv6-daemon-umip-0.3-fix/src/movement.c
--- mipv6-daemon-umip-0.3/src/movement.c	2007-03-29 16:51:31.000000000 +0200
+++ mipv6-daemon-umip-0.3-fix/src/movement.c	2007-06-13 16:05:55.000000000 +0200
@@ -74,15 +74,15 @@
 static pthread_t md_listener;
 
 static int conf_default_autoconf = 1;
-static int conf_default_ra = 1;
+/*static int conf_default_ra = 1;*/
 static int conf_default_ra_defrtr = 1;
-static int conf_default_ra_pinfo = 1;
+/*static int conf_default_ra_pinfo = 1;*/
 static int conf_default_rs = 3;
 static int conf_default_rs_ival = 4;
 
 static int conf_autoconf = 1;
-static int conf_ra = 1;
-static int conf_ra_pinfo = 1;
+/*static int conf_ra = 1;*/
+/*static int conf_ra_pinfo = 1;*/
 static int conf_ra_defrtr = 0;
 static int conf_rs = 0;
 
@@ -286,7 +286,7 @@
 
 static void md_block_rule_del(struct md_inet6_iface *iface)
 {
-	rule_del(NULL, 0, IP6_RULE_PRIO_MIP6_BLOCK, RTN_BLACKHOLE,
+	rule_del(NULL, 0, IP6_RULE_PRIO_MIP6_BLOCK_HOA, RTN_BLACKHOLE,
 		 &in6addr_any, 0, &in6addr_any, 0, 0);
 	rule_del(NULL, RT6_TABLE_MAIN, IP6_RULE_PRIO_MIP6_COA_OUT, RTN_UNICAST,
 		 &in6addr_any, 128, &in6addr_any, 0, 0);
@@ -818,7 +818,7 @@
 	return 0;
 }
 
-static int process_nlmsg(struct sockaddr_nl *who,
+static int process_nlmsg(const struct sockaddr_nl *who,
 			 struct nlmsghdr *n, void *arg)
 {
 	pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, NULL);
@@ -1332,7 +1332,7 @@
 	   on CoA to make routing and tunnel end-point updates atomic
 	   during handoff */
 	return rule_add(NULL, 0,
-			IP6_RULE_PRIO_MIP6_BLOCK, RTN_BLACKHOLE,
+			IP6_RULE_PRIO_MIP6_BLOCK_HOA, RTN_BLACKHOLE,
 			&in6addr_any, 0, &in6addr_any, 0, 0);
 }
 
diff -Nur mipv6-daemon-umip-0.3/src/rtnl.h mipv6-daemon-umip-0.3-fix/src/rtnl.h
--- mipv6-daemon-umip-0.3/src/rtnl.h	2007-03-29 16:51:31.000000000 +0200
+++ mipv6-daemon-umip-0.3-fix/src/rtnl.h	2007-06-13 16:05:55.000000000 +0200
@@ -19,7 +19,8 @@
 #define IP6_RULE_PRIO_MIP6_HOA_OUT   1001
 #define IP6_RULE_PRIO_MIP6_COA_OUT   1002
 #define IP6_RULE_PRIO_MIP6_BLOCK     1003
-#define IP6_RULE_PRIO_MIP6_FWD       1004
+#define IP6_RULE_PRIO_MIP6_BLOCK_HOA 1004
+#define IP6_RULE_PRIO_MIP6_FWD       1005
 
 static inline int rtnl_route_open(struct rtnl_handle *rth, 
 				  unsigned subscriptions)
makefile-gnu-source.diff (application/octet-stream, 604 B)
diff -Nur mipv6-daemon-umip-0.3/src/Makefile.am mipv6-daemon-umip-0.3-fix/src/Makefile.am
--- mipv6-daemon-umip-0.3/src/Makefile.am	2007-03-29 16:51:31.000000000 +0200
+++ mipv6-daemon-umip-0.3-fix/src/Makefile.am	2007-06-13 16:05:55.000000000 +0200
@@ -1,7 +1,7 @@
 ## $Id: Makefile.am 1.55 06/05/07 17:37:11+03:00 [email protected] $
 ## Process this file with automake to produce Makefile.in
 
-AM_CFLAGS = -Wall -Wpointer-arith -Wreturn-type -pedantic -std=gnu99
+AM_CFLAGS = -Wall -Wpointer-arith -Wreturn-type -pedantic -std=gnu99 -D_GNU_SOURCE
 AM_CPPFLAGS = -isystem ../include
 AM_YFLAGS = -d
rth2-nakamura-fix.diff (application/octet-stream, 3.8 KB)
diff -Nur mipv6-daemon-umip-0.3/config.h.in mipv6-daemon-umip-0.3-fix/config.h.in
--- mipv6-daemon-umip-0.3/config.h.in	2007-03-29 16:51:31.000000000 +0200
+++ mipv6-daemon-umip-0.3-fix/config.h.in	2007-06-13 16:05:55.000000000 +0200
@@ -10,6 +10,10 @@
    you don't. */
 #undef HAVE_DECL_IFA_F_HOMEADDRESS
 
+/* Define to 1 if you have the declaration of `IPV6_RTHDR_TYPE_2', and to 0 if
+   you don't. */
+#undef HAVE_DECL_IPV6_RTHDR_TYPE_2
+
 /* Define to 1 if you don't have `vprintf' but do have `_doprnt.' */
 #undef HAVE_DOPRNT
 
@@ -250,7 +254,7 @@
    supported. Do not define if restrict is supported directly. */
 #undef restrict
 
-/* Define to `unsigned' if <sys/types.h> does not define. */
+/* Define to `unsigned int' if <sys/types.h> does not define. */
 #undef size_t
 
 /* Define as `fork' if `vfork' does not work. */
diff -Nur mipv6-daemon-umip-0.3/configure.ac mipv6-daemon-umip-0.3-fix/configure.ac
--- mipv6-daemon-umip-0.3/configure.ac	2007-03-29 16:51:31.000000000 +0200
+++ mipv6-daemon-umip-0.3-fix/configure.ac	2007-06-13 16:05:55.000000000 +0200
@@ -130,22 +130,34 @@
 	       [ac_cv_enable_vt], [ac_cv_enable_vt=no])
 AM_CONDITIONAL(ENABLE_VT, test x$enable_vt = xyes)
 
-dnl Check Advanced IPv6 Socket API function availability
-AC_MSG_NOTICE([*** checking for RFC3542: Advanced IPv6 Socket API support])
+dnl Check Advanced IPv6 Socket API destination options header function availability
+AC_MSG_NOTICE([*** checking inet6_opt for RFC3542: Advanced IPv6 Socket API support])
 AC_REPLACE_FUNCS(inet6_opt_find)
-AC_REPLACE_FUNCS(inet6_rth_space)
-AC_REPLACE_FUNCS(inet6_rth_init)
-AC_REPLACE_FUNCS(inet6_rth_add)
-AC_REPLACE_FUNCS(inet6_rth_getaddr)
-AC_REPLACE_FUNCS(inet6_rth_gettype)
+
+dnl Check Advanced IPv6 Socket API routing header function availability
+AC_MSG_NOTICE([*** checking inet6_rth for RFC3542: Advanced IPv6 Socket API support])
+dnl Check Extension to Socket API for MIPv6 availability
+AC_MSG_NOTICE([*** checking RH2 for RFC4584: Extension to Socket API for MIPv6])
+AC_CHECK_DECLS([IPV6_RTHDR_TYPE_2],
+	[AC_REPLACE_FUNCS(inet6_rth_space)
+	 AC_REPLACE_FUNCS(inet6_rth_init)
+	 AC_REPLACE_FUNCS(inet6_rth_add)
+	 AC_REPLACE_FUNCS(inet6_rth_getaddr)
+	 AC_REPLACE_FUNCS(inet6_rth_gettype)],
+	[AC_LIBOBJ(inet6_rth_space)
+	 AC_LIBOBJ(inet6_rth_init)
+	 AC_LIBOBJ(inet6_rth_add)
+	 AC_LIBOBJ(inet6_rth_getaddr)
+	 AC_LIBOBJ(inet6_rth_gettype)],)
+
 dnl Check some functions are really missing so we don't build empty libmissing
 AM_CONDITIONAL(MISSING,
-   test x$ac_cv_func_inet6_opt_find = xno -o \
-	x$ac_cv_func_inet6_rth_space = xno -o \
-	x$ac_cv_func_inet6_rth_init = xno -o \
-	x$ac_cv_func_inet6_rth_add = xno -o \
-	x$ac_cv_func_inet6_rth_getaddr = xno -o \
-	x$ac_cv_func_inet6_rth_gettype = xno)
+   test x$ac_cv_func_inet6_opt_find != xyes -o \
+	x$ac_cv_func_inet6_rth_space != xyes -o \
+	x$ac_cv_func_inet6_rth_init != xyes -o \
+	x$ac_cv_func_inet6_rth_add != xyes -o \
+	x$ac_cv_func_inet6_rth_getaddr != xyes -o \
+	x$ac_cv_func_inet6_rth_gettype != xyes)
 
 AC_MSG_NOTICE([*** checking for Mobile IPv6 support in kernel headers])
 AC_CHECK_DECLS([IFA_F_HOMEADDRESS],,
diff -Nur mipv6-daemon-umip-0.3/src/mh.c mipv6-daemon-umip-0.3-fix/src/mh.c
--- mipv6-daemon-umip-0.3/src/mh.c	2007-03-29 16:51:31.000000000 +0200
+++ mipv6-daemon-umip-0.3-fix/src/mh.c	2007-06-13 16:05:55.000000000 +0200
@@ -619,6 +619,10 @@
 	cmsglen = CMSG_SPACE(sizeof(pinfo));
 	if (addrs->remote_coa != NULL) {
 		rthlen = inet6_rth_space(IPV6_RTHDR_TYPE_2, 1);
+		if (!rthlen) {
+			MDBG("inet6_rth_space error\n");
+			return -1;
+		}
 		cmsglen += CMSG_SPACE(rthlen);
 	}
 	cmsg = malloc(cmsglen);
@@ -658,6 +662,7 @@
 		rthp = inet6_rth_init(rthp, rthlen, IPV6_RTHDR_TYPE_2, 1);
 		if (rthp == NULL) {
 			free(msg.msg_control);
+			MDBG("inet6_rth_init error\n");
 			return -3;
 		}
 		inet6_rth_add(rthp, addrs->remote_coa);
xfrm.c.diff (application/octet-stream, 1.8 KB)
diff -Nur mipv6-daemon-umip-0.3/src/xfrm.c mipv6-daemon-umip-0.3-fix/src/xfrm.c
--- mipv6-daemon-umip-0.3/src/xfrm.c	2007-03-29 16:51:31.000000000 +0200
+++ mipv6-daemon-umip-0.3-fix/src/xfrm.c	2007-06-13 16:05:55.000000000 +0200
@@ -1968,9 +1968,11 @@
 	hai->home_block |= HOME_LINK_BLOCK;
 	hai->if_block = hai->hoa.iif;
 	/* allow MN to send NA messages to HA while returning home */
+	/* RK: such policy is already installed in xfrm_cn_init at startup,
+	 * so we update it (update field to 1) */
 	set_selector(&in6addr_any, &in6addr_any, IPPROTO_ICMPV6,
 		     ND_NEIGHBOR_ADVERT, 0, hai->if_block, &sel);
-	if ((ret = xfrm_mip_policy_add(&sel, 0, XFRM_POLICY_OUT, XFRM_POLICY_ALLOW,
+	if ((ret = xfrm_mip_policy_add(&sel, 1 /* update */, XFRM_POLICY_OUT, XFRM_POLICY_ALLOW,
 				       MIP6_PRIO_HOME_SIG_ANY, NULL, 0)))
 		return ret;
 	/* block any packets from HoA to the CN */
@@ -1987,9 +1989,16 @@
 	struct xfrm_selector sel;
 	set_selector(&in6addr_any, &in6addr_any, 0, 0, 0, hai->if_block, &sel);
 	xfrm_mip_policy_del(&sel, XFRM_POLICY_OUT);
-	set_selector(&in6addr_any, &in6addr_any, IPPROTO_ICMPV6,
-		     ND_NEIGHBOR_ADVERT, 0, hai->if_block, &sel);
-	xfrm_mip_policy_del(&sel, XFRM_POLICY_OUT);
+	/* RK: instead of deleting the policy, update it to its previous state (
+	 * the one installed in xfrm_cn_init) */
+	//set_selector(&in6addr_any, &in6addr_any, IPPROTO_ICMPV6,
+	//	     ND_NEIGHBOR_ADVERT, 0, hai->if_block, &sel);
+	//xfrm_mip_policy_del(&sel, XFRM_POLICY_OUT);
+	set_selector(&in6addr_any, &in6addr_any,
+		     IPPROTO_ICMPV6, ND_NEIGHBOR_ADVERT, 0, 0, &sel);
+	if (xfrm_mip_policy_add(&sel, 1 /* update */, XFRM_POLICY_OUT, XFRM_POLICY_ALLOW,
+				MIP6_PRIO_NO_RO_SIG_ANY, NULL, 0) < 0)
+		XDBG("Could not update NA policy\n");
 	hai->if_block = 0;
 	hai->home_block &= ~HOME_LINK_BLOCK;
 }