Re: svn commit: r197210 - in head/sys: netinet nfsclient

2009-09-15 Thread Qing Li
The users who were reporting the NFS mount problems confirm the patch fixes
their issues. At the moment I don't know why the patch would break NFS ROOT.
I just backed out the change so RC1 can be made but I will continue the
investigation.

--Qing

On Tue, Sep 15, 2009 at 3:55 PM, Rick Macklem  wrote:
>
>
> On Tue, 15 Sep 2009, Bjoern A. Zeeb wrote:
>
>> On Tue, 15 Sep 2009, Qing Li wrote:
>>
>>> Author: qingli
>>> Date: Tue Sep 15 01:01:03 2009
>>> New Revision: 197210
>>> URL: http://svn.freebsd.org/changeset/base/197210
>>>
>>> Log:
>>>  The bootp code installs an interface address and the nfs client
>>>  module tries to install the same address again. This extra code
>>>  is removed, which was discovered by the removal of a call to
>>>  in_ifscrub() in r196714. This call to in_ifscrub is put back here
>>>  because the SIOCAIFADDR command can be used to change the prefix
>>>  length of an existing alias.
>>>
>>>  Reviewed by:    kmacy
>>
>> This broke NFS Root for me in the netperf clsuter setup.
>> The NFS Root mount hang for ages (I reset the box after 1 hour).
>>
>> Backing out r197212 and this and it boots just fine again.
>>
> I don't know diddly about diskless booting and have no setup to test,
> but if I understood the problem, might something like the following
> work?
>
> rick
> --- nfsclient/nfs_vfsops.c.sav  2009-09-15 18:39:32.0 -0400
> +++ nfsclient/nfs_vfsops.c      2009-09-15 18:41:52.0 -0400
> @@ -416,13 +416,14 @@
>        struct socket *so;
>        struct vnode *vp;
>        struct ifreq ir;
> -       int error;
> +       int error, doioctl = 1;
>        u_long l;
>        char buf[128];
>        char *cp;
>
>  #if defined(BOOTP_NFSROOT) && defined(BOOTP)
>        bootpc_init();          /* use bootp to get nfs_diskless filled in */
> +       doioctl = 0;
>  #elif defined(NFS_ROOT)
>        nfs_setup_diskless();
>  #endif
> @@ -463,9 +464,11 @@
>                        break;
>        }
>  #endif
> -       error = ifioctl(so, SIOCAIFADDR, (caddr_t)&nd->myif, td);
> -       if (error)
> -               panic("nfs_mountroot: SIOCAIFADDR: %d", error);
> +       if (doioctl) {
> +               error = ifioctl(so, SIOCAIFADDR, (caddr_t)&nd->myif, td);
> +               if (error)
> +                       panic("nfs_mountroot: SIOCAIFADDR: %d", error);
> +       }
>        if ((cp = getenv("boot.netif.mtu")) != NULL) {
>                ir.ifr_mtu = strtol(cp, NULL, 10);
>                bcopy(nd->myif.ifra_name, ir.ifr_name, IFNAMSIZ);
>
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r197210 - in head/sys: netinet nfsclient

2009-09-15 Thread Rick Macklem



On Tue, 15 Sep 2009, Bjoern A. Zeeb wrote:


On Tue, 15 Sep 2009, Qing Li wrote:


Author: qingli
Date: Tue Sep 15 01:01:03 2009
New Revision: 197210
URL: http://svn.freebsd.org/changeset/base/197210

Log:
 The bootp code installs an interface address and the nfs client
 module tries to install the same address again. This extra code
 is removed, which was discovered by the removal of a call to
 in_ifscrub() in r196714. This call to in_ifscrub is put back here
 because the SIOCAIFADDR command can be used to change the prefix
 length of an existing alias.

 Reviewed by:kmacy


This broke NFS Root for me in the netperf clsuter setup.
The NFS Root mount hang for ages (I reset the box after 1 hour).

Backing out r197212 and this and it boots just fine again.


I don't know diddly about diskless booting and have no setup to test,
but if I understood the problem, might something like the following
work?

rick
--- nfsclient/nfs_vfsops.c.sav  2009-09-15 18:39:32.0 -0400
+++ nfsclient/nfs_vfsops.c  2009-09-15 18:41:52.0 -0400
@@ -416,13 +416,14 @@
struct socket *so;
struct vnode *vp;
struct ifreq ir;
-   int error;
+   int error, doioctl = 1;
u_long l;
char buf[128];
char *cp;

 #if defined(BOOTP_NFSROOT) && defined(BOOTP)
bootpc_init();  /* use bootp to get nfs_diskless filled in */
+   doioctl = 0;
 #elif defined(NFS_ROOT)
nfs_setup_diskless();
 #endif
@@ -463,9 +464,11 @@
break;
}
 #endif
-   error = ifioctl(so, SIOCAIFADDR, (caddr_t)&nd->myif, td);
-   if (error)
-   panic("nfs_mountroot: SIOCAIFADDR: %d", error);
+   if (doioctl) {
+   error = ifioctl(so, SIOCAIFADDR, (caddr_t)&nd->myif, td);
+   if (error)
+   panic("nfs_mountroot: SIOCAIFADDR: %d", error);
+   }
if ((cp = getenv("boot.netif.mtu")) != NULL) {
ir.ifr_mtu = strtol(cp, NULL, 10);
bcopy(nd->myif.ifra_name, ir.ifr_name, IFNAMSIZ);
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r197210 - in head/sys: netinet nfsclient

2009-09-15 Thread Bjoern A. Zeeb

On Tue, 15 Sep 2009, Qing Li wrote:


Author: qingli
Date: Tue Sep 15 01:01:03 2009
New Revision: 197210
URL: http://svn.freebsd.org/changeset/base/197210

Log:
 The bootp code installs an interface address and the nfs client
 module tries to install the same address again. This extra code
 is removed, which was discovered by the removal of a call to
 in_ifscrub() in r196714. This call to in_ifscrub is put back here
 because the SIOCAIFADDR command can be used to change the prefix
 length of an existing alias.

 Reviewed by:kmacy


This broke NFS Root for me in the netperf clsuter setup.
The NFS Root mount hang for ages (I reset the box after 1 hour).

Backing out r197212 and this and it boots just fine again.



Modified:
 head/sys/netinet/in.c
 head/sys/nfsclient/nfs_vfsops.c

Modified: head/sys/netinet/in.c
==
--- head/sys/netinet/in.c   Tue Sep 15 00:26:23 2009(r197209)
+++ head/sys/netinet/in.c   Tue Sep 15 01:01:03 2009(r197210)
@@ -536,6 +536,16 @@ in_control(struct socket *so, u_long cmd
hostIsNew = 0;
}
if (ifra->ifra_mask.sin_len) {
+   /*
+* QL: XXX
+* Need to scrub the prefix here in case
+* the issued command is SIOCAIFADDR with
+* the same address, but with a different
+* prefix length. And if the prefix length
+* is the same as before, then the call is
+* un-necessarily executed here.
+*/
+   in_ifscrub(ifp, ia);
ia->ia_sockmask = ifra->ifra_mask;
ia->ia_sockmask.sin_family = AF_INET;
ia->ia_subnetmask =
@@ -544,6 +554,7 @@ in_control(struct socket *so, u_long cmd
}
if ((ifp->if_flags & IFF_POINTOPOINT) &&
(ifra->ifra_dstaddr.sin_family == AF_INET)) {
+   in_ifscrub(ifp, ia);
ia->ia_dstaddr = ifra->ifra_dstaddr;
maskIsNew  = 1; /* We lie; but the effect's the same */
}

Modified: head/sys/nfsclient/nfs_vfsops.c
==
--- head/sys/nfsclient/nfs_vfsops.c Tue Sep 15 00:26:23 2009
(r197209)
+++ head/sys/nfsclient/nfs_vfsops.c Tue Sep 15 01:01:03 2009
(r197210)
@@ -463,9 +463,13 @@ nfs_mountroot(struct mount *mp)
break;
}
#endif
+
+#if 0 /* QL: XXX */
error = ifioctl(so, SIOCAIFADDR, (caddr_t)&nd->myif, td);
if (error)
panic("nfs_mountroot: SIOCAIFADDR: %d", error);
+#endif
+
if ((cp = getenv("boot.netif.mtu")) != NULL) {
ir.ifr_mtu = strtol(cp, NULL, 10);
bcopy(nd->myif.ifra_name, ir.ifr_name, IFNAMSIZ);



--
Bjoern A. Zeeb   What was I talking about and who are you again?
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


svn commit: r197210 - in head/sys: netinet nfsclient

2009-09-14 Thread Qing Li
Author: qingli
Date: Tue Sep 15 01:01:03 2009
New Revision: 197210
URL: http://svn.freebsd.org/changeset/base/197210

Log:
  The bootp code installs an interface address and the nfs client
  module tries to install the same address again. This extra code
  is removed, which was discovered by the removal of a call to
  in_ifscrub() in r196714. This call to in_ifscrub is put back here
  because the SIOCAIFADDR command can be used to change the prefix
  length of an existing alias.
  
  Reviewed by:kmacy

Modified:
  head/sys/netinet/in.c
  head/sys/nfsclient/nfs_vfsops.c

Modified: head/sys/netinet/in.c
==
--- head/sys/netinet/in.c   Tue Sep 15 00:26:23 2009(r197209)
+++ head/sys/netinet/in.c   Tue Sep 15 01:01:03 2009(r197210)
@@ -536,6 +536,16 @@ in_control(struct socket *so, u_long cmd
hostIsNew = 0;
}
if (ifra->ifra_mask.sin_len) {
+   /* 
+* QL: XXX
+* Need to scrub the prefix here in case
+* the issued command is SIOCAIFADDR with
+* the same address, but with a different
+* prefix length. And if the prefix length
+* is the same as before, then the call is 
+* un-necessarily executed here.
+*/
+   in_ifscrub(ifp, ia);
ia->ia_sockmask = ifra->ifra_mask;
ia->ia_sockmask.sin_family = AF_INET;
ia->ia_subnetmask =
@@ -544,6 +554,7 @@ in_control(struct socket *so, u_long cmd
}
if ((ifp->if_flags & IFF_POINTOPOINT) &&
(ifra->ifra_dstaddr.sin_family == AF_INET)) {
+   in_ifscrub(ifp, ia);
ia->ia_dstaddr = ifra->ifra_dstaddr;
maskIsNew  = 1; /* We lie; but the effect's the same */
}

Modified: head/sys/nfsclient/nfs_vfsops.c
==
--- head/sys/nfsclient/nfs_vfsops.c Tue Sep 15 00:26:23 2009
(r197209)
+++ head/sys/nfsclient/nfs_vfsops.c Tue Sep 15 01:01:03 2009
(r197210)
@@ -463,9 +463,13 @@ nfs_mountroot(struct mount *mp)
break;
}
 #endif
+
+#if 0 /* QL: XXX */
error = ifioctl(so, SIOCAIFADDR, (caddr_t)&nd->myif, td);
if (error)
panic("nfs_mountroot: SIOCAIFADDR: %d", error);
+#endif
+
if ((cp = getenv("boot.netif.mtu")) != NULL) {
ir.ifr_mtu = strtol(cp, NULL, 10);
bcopy(nd->myif.ifra_name, ir.ifr_name, IFNAMSIZ);
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"