Linux NFS development
 help / color / mirror / Atom feed
From: "J. Bruce Fields" <bfields@fieldses.org>
To: Kinglong Mee <kinglongmee@gmail.com>
Cc: "linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>,
	tigran.mkrtchyan@desy.de
Subject: Re: [PATCH 1/4] 4.0 setclientid: More cases for setclientid behavior
Date: Wed, 29 Jul 2015 15:07:38 -0400	[thread overview]
Message-ID: <20150729190738.GA21742@fieldses.org> (raw)
In-Reply-To: <55B76B94.4060208@gmail.com>

On Tue, Jul 28, 2015 at 07:46:28PM +0800, Kinglong Mee wrote:
> More cases of RFC 7530 16.33.5 setclientid/setclientid_confirm behavior.

Thanks for doing these!  Just one nit:

> 
> Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
> ---
>  nfs4.0/servertests/st_setclientid.py | 245 ++++++++++++++++++++++++++++++++---
>  1 file changed, 227 insertions(+), 18 deletions(-)
> 
> diff --git a/nfs4.0/servertests/st_setclientid.py b/nfs4.0/servertests/st_setclientid.py
> index e9d0052..ca3e1d4 100644
> --- a/nfs4.0/servertests/st_setclientid.py
> +++ b/nfs4.0/servertests/st_setclientid.py
> @@ -49,25 +49,7 @@ def testClientUpdateCallback(t, env):
>      c.init_connection(id, verf)
>      res = c.close_file(t.code, fh, stateid)
>      check(res, msg="Close after updating callback info")
> -    
> -def testInUse(t, env):
> -    """SETCLIENTID with same nfs_client_id.id should return NFS4ERR_CLID_INUSE
>  
> -    This requires NCL1 and NCL2 to have different principals (UIDs).
> -    
> -    FLAGS: setclientid setclientidconfirm all
> -    DEPEND: _checkprinciples INIT
> -    CODE: CID2
> -    """
> -    c1 = env.c1
> -    c2 = env.c2
> -    c1.init_connection("Badid_for_%s_pid=%i" % (t.code, os.getpid()),
> -                       verifier=c1.verifier)
> -    ops = [c2.setclientid(id="Badid_for_%s_pid=%i" % (t.code, os.getpid()),
> -                          verifier=c1.verifier)]
> -    res = c2.compound(ops)
> -    check(res, NFS4ERR_CLID_INUSE, "SETCLIENTID with same nfs_client_id.id")
> -    
>  def testLoseAnswer(t, env):
>      """SETCLIENTID after a client reboot could cause case not covered in RFC
>  
> @@ -114,7 +96,234 @@ def testAllCases(t, env):
>      # (*xc*s), (*xd*t)
>      res = c.compound([c.setclientid(id=id, verifier='')])
>      check(res)
> +
> +def testInUse(t, env):
> +    """SETCLIENTID with same nfs_client_id.id should return NFS4ERR_CLID_INUSE
> +
> +    This requires NCL1 and NCL2 to have different principals (UIDs).
>      
> +    FLAGS: setclientid setclientidconfirm all
> +    DEPEND: _checkprinciples INIT
> +    CODE: CID4a

So CID4a is more-or-less the same as CID2?

I'd rather not change the test code from CID2 to CID4a.   That kind of
change makes it hard to compare results across different versions of
pynfs.

Changes look fine otherwise.

--b.


> +    """
> +    c1 = env.c1
> +    c2 = env.c2
> +    clid = "Clid_for_%s_pid=%i" % (t.code, os.getpid())
> +    c1.init_connection(clid, verifier=c1.verifier)
> +    ops = [c2.setclientid(clid, verifier=c1.verifier)]
> +    res = c2.compound(ops)
> +    check(res, NFS4ERR_CLID_INUSE, "SETCLIENTID with same nfs_client_id.id")
> +
> +def testCallbackInfoUpdate(t, env):
> +    """A probable callback information update and records
> +       an unconfirmed { v, x, c, k, t } and leaves the
> +       confirmed { v, x, c, l, s } in place, such that t != s.
> +
> +    FLAGS: setclientid all
> +    DEPEND: INIT
> +    CODE: CID4b
> +    """
> +    c1 = env.c1
> +    clid = "Clid_for_%s_pid=%i" % (t.code, os.getpid())
> +
> +    # confirmed { v, x, c, l, s }
> +    (cclientid, cconfirm) = c1.init_connection(clid, verifier=c1.verifier)
> +
> +    # request { v, x, c, k, s } --> unconfirmed { v, x, c, k, t }
> +    ops = [c1.setclientid(clid, verifier=c1.verifier)]
> +    res = c1.compound(ops)
> +    check(res)
> +
> +    tclientid = res.resarray[0].switch.switch.clientid
> +    tconfirm = res.resarray[0].switch.switch.setclientid_confirm
> +
> +    # (t != s)
> +    if tconfirm == '\x00\x00\x00\x00\x00\x00\x00\x00':
> +        t.fail("Got clientid confirm verifier with all zero!")
> +
> +    if cclientid != tclientid:
> +        t.fail("Return a different clientID for callback information updating!")
> +
> +    if tconfirm == cconfirm:
> +        t.fail("Return a same confirm for callback information updating!")
> +
> +def testConfirmedDiffVerifier(t, env):
> +    """The server has previously recorded a confirmed { u, x, c, l, s }
> +       record such that v != u, l may or may not equal k, and has not
> +       recorded any unconfirmed { *, x, *, *, * } record for x.  The
> +       server records an unconfirmed { v, x, d, k, t } (d != c, t != s).
> +
> +    FLAGS: setclientid all
> +    DEPEND: INIT
> +    CODE: CID4c
> +    """
> +    c1 = env.c1
> +    clid = "Clid_for_%s_pid=%i" % (t.code, os.getpid())
> +
> +    # confirmed { u, x, c, l, s }
> +    (cclientid, cconfirm) = c1.init_connection(clid, verifier=c1.verifier)
> +
> +    # request { v, x, c, k, s } --> unconfirmed { v, x, d, k, t }
> +    ops = [c1.setclientid(clid, verifier="diff")]
> +    res = c1.compound(ops)
> +    check(res)
> +
> +    tclientid = res.resarray[0].switch.switch.clientid
> +    tconfirm = res.resarray[0].switch.switch.setclientid_confirm
> +
> +    # (d != c, t != s)
> +    if tconfirm == '\x00\x00\x00\x00\x00\x00\x00\x00':
> +        t.fail("Got clientid confirm verifier with all zero!")
> +
> +    if cclientid == tclientid:
> +        t.fail("Return a same clientID for different verifier!")
> +
> +    if tconfirm == cconfirm:
> +        t.fail("Return a same confirm for different verifier!")
> +
> +def testConfUnConfDiffVerifier1(t, env):
> +    """The server has previously recorded a confirmed { u, x, c, l, s }
> +       record such that v != u, l may or may not equal k, and recorded an
> +       unconfirmed { w, x, d, m, t } record such that c != d, t != s, m
> +       may or may not equal k, m may or may not equal l, and k may or may
> +       not equal l.  Whether w == v or w != v makes no difference.  The
> +       server simply removes the unconfirmed { w, x, d, m, t } record and
> +       replaces it with an unconfirmed { v, x, e, k, r } record, such
> +       that e != d, e != c, r != t, r != s.
> +
> +    FLAGS: setclientid all
> +    DEPEND: INIT
> +    CODE: CID4d1
> +    """
> +    c1 = env.c1
> +    clid = "Clid_for_%s_pid=%i" % (t.code, os.getpid())
> +
> +    # confirmed { u, x, c, l, s }
> +    (cclientid, cconfirm) = c1.init_connection(clid, verifier=c1.verifier)
> +
> +    # unconfirmed { w, x, d, m, t }
> +    ops = [c1.setclientid(clid, verifier="unconf")]
> +    res = c1.compound(ops)
> +    check(res)
> +
> +    uclientid = res.resarray[0].switch.switch.clientid
> +    uconfirm = res.resarray[0].switch.switch.setclientid_confirm
> +
> +    # request { v, x, c, k, s } --> unconfirmed { v, x, e, k, r }
> +    # (v == w)
> +    ops = [c1.setclientid(clid, verifier="unconf")]
> +    res = c1.compound(ops)
> +    check(res)
> +
> +    tclientid = res.resarray[0].switch.switch.clientid
> +    tconfirm = res.resarray[0].switch.switch.setclientid_confirm
> +
> +    # removes the unconfirmed { w, x, d, m, t }
> +    ops = [c1.setclientid_confirm_op(uclientid, uconfirm)]
> +    res = c1.compound(ops)
> +    check(res, NFS4ERR_STALE_CLIENTID)
> +
> +    # (e != d, e != c, r != t, r != s)
> +    if tconfirm == '\x00\x00\x00\x00\x00\x00\x00\x00':
> +        t.fail("Got clientid confirm verifier with all zero!")
> +
> +    if cclientid == tclientid or uclientid == tclientid:
> +        t.fail("Return a same clientID for different verifier!")
> +
> +    if tconfirm == cconfirm or tconfirm == uconfirm:
> +        t.fail("Return a same confirm for different verifier!")
> +
> +def testConfUnConfDiffVerifier2(t, env):
> +    """Whether w == v or w != v makes no difference.
> +
> +    FLAGS: setclientid all
> +    DEPEND: INIT
> +    CODE: CID4d2
> +    """
> +    c1 = env.c1
> +    clid = "Clid_for_%s_pid=%i" % (t.code, os.getpid())
> +
> +    # confirmed { u, x, c, l, s }
> +    (cclientid, cconfirm) = c1.init_connection(clid, verifier=c1.verifier)
> +
> +    # unconfirmed { w, x, d, m, t }
> +    ops = [c1.setclientid(clid, verifier="unconf")]
> +    res = c1.compound(ops)
> +    check(res)
> +
> +    uclientid = res.resarray[0].switch.switch.clientid
> +    uconfirm = res.resarray[0].switch.switch.setclientid_confirm
> +
> +    # request { v, x, c, k, s } --> unconfirmed { v, x, e, k, r }
> +    # (v != w)
> +    ops = [c1.setclientid(clid, verifier="testconf")]
> +    res = c1.compound(ops)
> +    check(res)
> +
> +    tclientid = res.resarray[0].switch.switch.clientid
> +    tconfirm = res.resarray[0].switch.switch.setclientid_confirm
> +
> +    # removes the unconfirmed { w, x, d, m, t }
> +    ops = [c1.setclientid_confirm_op(uclientid, uconfirm)]
> +    res = c1.compound(ops)
> +    check(res, NFS4ERR_STALE_CLIENTID)
> +
> +    # (e != d, e != c, r != t, r != s)
> +    if tconfirm == '\x00\x00\x00\x00\x00\x00\x00\x00':
> +        t.fail("Got clientid confirm verifier with all zero!")
> +
> +    if cclientid == tclientid or uclientid == tclientid:
> +        t.fail("Return a same clientID for different verifier!")
> +
> +    if tconfirm == cconfirm or tconfirm == uconfirm:
> +        t.fail("Return a same confirm for different verifier!")
> +
> +def testUnConfReplaced(t, env):
> +    """The server has no confirmed { *, x, *, *, * } for x.  It may or
> +       may not have recorded an unconfirmed { u, x, c, l, s }, where l
> +       may or may not equal k, and u may or may not equal v.  Any
> +       unconfirmed record { u, x, c, l, * }, regardless of whether u == v
> +       or l == k, is replaced with an unconfirmed record { v, x, d, k, t}
> +       where d != c, t != s.
> +
> +    FLAGS: setclientid all
> +    DEPEND: INIT
> +    CODE: CID4e
> +    """
> +    c1 = env.c1
> +    clid = "Clid_for_%s_pid=%i" % (t.code, os.getpid())
> +
> +    # unconfirmed { w, x, d, m, t }
> +    ops = [c1.setclientid(clid, verifier="unconf")]
> +    res = c1.compound(ops)
> +    check(res)
> +
> +    uclientid = res.resarray[0].switch.switch.clientid
> +    uconfirm = res.resarray[0].switch.switch.setclientid_confirm
> +
> +    # request { v, x, c, k, s } --> unconfirmed { v, x, d, k, t }
> +    ops = [c1.setclientid(clid, verifier="diff")]
> +    res = c1.compound(ops)
> +    check(res)
> +
> +    tclientid = res.resarray[0].switch.switch.clientid
> +    tconfirm = res.resarray[0].switch.switch.setclientid_confirm
> +
> +    # removes the unconfirmed { w, x, d, m, t }
> +    ops = [c1.setclientid_confirm_op(uclientid, uconfirm)]
> +    res = c1.compound(ops)
> +    check(res, NFS4ERR_STALE_CLIENTID)
> +
> +    # (d != c, t != s)
> +    if tconfirm == '\x00\x00\x00\x00\x00\x00\x00\x00':
> +        t.fail("Got clientid confirm verifier with all zero!")
> +
> +    if uclientid == tclientid:
> +        t.fail("Return a same clientID for different verifier!")
> +
> +    if tconfirm == uconfirm:
> +        t.fail("Return a same confirm for different verifier!")
> +
>  def testLotsOfClients(t, env):
>      """SETCLIENTID called multiple times
>  
> -- 
> 2.4.3

  reply	other threads:[~2015-07-29 19:07 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-28 11:46 [PATCH 1/4] 4.0 setclientid: More cases for setclientid behavior Kinglong Mee
2015-07-29 19:07 ` J. Bruce Fields [this message]
2015-07-30  9:18   ` [PATCH v2] " Kinglong Mee
2015-07-30 15:38     ` J. Bruce Fields

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150729190738.GA21742@fieldses.org \
    --to=bfields@fieldses.org \
    --cc=kinglongmee@gmail.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=tigran.mkrtchyan@desy.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox