[SCM] Samba Shared Repository - branch v3-6-test updated

Karolin Seeger kseeger at samba.org
Tue Jul 24 12:52:19 MDT 2012


The branch, v3-6-test has been updated
       via  8ce2065 s3:smb2_server: implement credit granting similar to windows
       via  09e26d6 s3:smb2_server: make sure sequence numbers don't wrap at UINT64_MAX
       via  c96dc10 s3:smb2_server: make sure we don't grant more credits than we allow
       via  c3559ee s3:smb2_server: check the already granted credits like in the master branch
       via  4391a14 s3:smb2_server: split out a smb2_validate_sequence_number() function
       via  d9d25de s3:smb2_server: clear sequence window if we got the lowest sequence id
       via  492f087 s3:smb2_server: fix calculation of the next bitmap_offset
       via  4bea48f s3:smb2_server: remove unused and confusing DEFAULT_SMB2_MAX_CREDIT_BITMAP_FACTOR
       via  aa29653 s3:smb2_server: call smbd_smb2_request_validate() also in smbd_smb2_first_negprot()
       via  138535d s3:smb2_server: start the connection with one credit granted to the client
      from  9c69a23 s3: Make us survive smb2.lock.rw-shared with aio enabled

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=v3-6-test


- Log -----------------------------------------------------------------
commit 8ce206519c34e95ec5222e50513dd253c673d11c
Author: Stefan Metzmacher <metze at samba.org>
Date:   Wed Jun 27 15:33:43 2012 +0200

    s3:smb2_server: implement credit granting similar to windows
    
    This makes it much easier to compare traces.
    
    metze
    (cherry picked from commit 648b959b13224105addaae483823bc422ed1cc21)
    
    The last 10 patches address bug #9057 - SMB2 credit handling code has bugs.

commit 09e26d6a177d82d7b73e632956f912edfcb874bb
Author: Stefan Metzmacher <metze at samba.org>
Date:   Wed Jun 27 15:33:43 2012 +0200

    s3:smb2_server: make sure sequence numbers don't wrap at UINT64_MAX
    
    metze
    (cherry picked from commit 82dc0b33b9af5094d78f3ecd855900e49c580343)

commit c96dc10849c52cae7d3bfddce212a2595563081e
Author: Stefan Metzmacher <metze at samba.org>
Date:   Wed Jun 27 15:33:43 2012 +0200

    s3:smb2_server: make sure we don't grant more credits than we allow
    
    If the client hasn't consumed the lowest seqnum, but the distance
    between lowest and highest seqnum has reached max credits.
    
    In that case we should stop granting credits.
    
    metze
    (similar to commit ee8ae459aea6879377b5510851a6dc673cf72aad)

commit c3559ee48fe3da59dba3b5afb69a45ef15475fa0
Author: Stefan Metzmacher <metze at samba.org>
Date:   Tue Jun 26 14:28:07 2012 +0200

    s3:smb2_server: check the already granted credits like in the master branch
    
    metze
    
    Backport of: s3:smb2_server: check the credit_charge against the already granted credits
    (similar to commit 4fe41c0bb14f6ae7e52aa7f180e66c7695eb6fa0)

commit 4391a14adca1c94016318b6890129f842b9ae1e7
Author: Stefan Metzmacher <metze at samba.org>
Date:   Mon Jun 25 23:17:55 2012 +0200

    s3:smb2_server: split out a smb2_validate_sequence_number() function
    
    metze
    (similar to commit 984fdaf9149d96d0d28600443981d87d13eb355c)

commit d9d25dedad4b64d5562f451b645e2dfab89d8c13
Author: Stefan Metzmacher <metze at samba.org>
Date:   Tue Jun 26 08:08:37 2012 +0200

    s3:smb2_server: clear sequence window if we got the lowest sequence id
    
    Otherwise we'll never consume sequence id '0'.
    
    metze
    (similar to commit d6e7a76461ad7582efa510676aa2bea230ea9f02)

commit 492f0878e49c141f31bb076c906c52131d7660ab
Author: Stefan Metzmacher <metze at samba.org>
Date:   Tue Jun 26 09:12:44 2012 +0200

    s3:smb2_server: fix calculation of the next bitmap_offset
    
    metze
    (similar to commit bd6d415cae550e97e04830eecefa2881b497de89)

commit 4bea48ff5a6c23818082b6b42401c72d892de51a
Author: Stefan Metzmacher <metze at samba.org>
Date:   Tue Jun 26 09:11:23 2012 +0200

    s3:smb2_server: remove unused and confusing DEFAULT_SMB2_MAX_CREDIT_BITMAP_FACTOR
    
    metze
    (similar to commit d1ee774ed0b4b3882b4b85da16d9bb9c082a0c49)

commit aa2965377a6156c74692829763f950f85a48f5f7
Author: Stefan Metzmacher <metze at samba.org>
Date:   Mon Jun 25 23:14:24 2012 +0200

    s3:smb2_server: call smbd_smb2_request_validate() also in smbd_smb2_first_negprot()
    
    We need to consume message_id 0, for SMB1 negprot starts.
    
    metze
    (cherry picked from commit 925994e42eba5b72ce605b68e8980adc1b5ecd83)

commit 138535dfc528ed66295991d10d063dbfd1a78ced
Author: Stefan Metzmacher <metze at samba.org>
Date:   Tue Jun 26 14:23:12 2012 +0200

    s3:smb2_server: start the connection with one credit granted to the client
    
    metze
    (cherry picked from commit 0b8eac9b79197c4659a5738f1b9399b3c88f2f8d)

-----------------------------------------------------------------------

Summary of changes:
 source3/include/local.h    |    1 -
 source3/smbd/globals.h     |   44 +++++++-
 source3/smbd/smb2_server.c |  258 +++++++++++++++++++++++++++++++------------
 3 files changed, 227 insertions(+), 76 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/include/local.h b/source3/include/local.h
index d659522..499f468 100644
--- a/source3/include/local.h
+++ b/source3/include/local.h
@@ -265,6 +265,5 @@
 #define DEFAULT_SMB2_MAX_WRITE (64*1024)
 #define DEFAULT_SMB2_MAX_TRANSACT (64*1024)
 #define DEFAULT_SMB2_MAX_CREDITS 8192
-#define DEFAULT_SMB2_MAX_CREDIT_BITMAP_FACTOR 2
 
 #endif
diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h
index 663daa4..eefc2c6 100644
--- a/source3/smbd/globals.h
+++ b/source3/smbd/globals.h
@@ -603,13 +603,51 @@ struct smbd_server_connection {
 			bool blocking_lock_unlock_state;
 		} locks;
 		struct smbd_smb2_request *requests;
+		/*
+		 * seqnum_low is the lowest sequence number
+		 * we will accept.
+		 */
 		uint64_t seqnum_low;
-		uint32_t credits_granted;
-		uint32_t max_credits;
+		/*
+		 * seqnum_range is the range of credits we have
+		 * granted from the sequence windows starting
+		 * at seqnum_low.
+		 *
+		 * This gets incremented when new credits are
+		 * granted and gets decremented when the
+		 * lowest sequence number is consumed
+		 * (when seqnum_low gets incremented).
+		 */
+		uint16_t seqnum_range;
+		/*
+		 * credits_grantedThe number of credits we have currently granted
+		 * to the client.
+		 *
+		 * This gets incremented when new credits are
+		 * granted and gets decremented when any credit
+		 * is comsumed.
+		 *
+		 * Note: the decrementing is different compared
+		 *       to seqnum_range.
+		 */
+		uint16_t credits_granted;
+		/*
+		 * The maximum number of credits we will ever
+		 * grant to the client.
+		 *
+		 * Typically we will only grant 1/16th of
+		 * max_credits.
+		 *
+		 * This is the "server max credits" parameter.
+		 */
+		uint16_t max_credits;
+		/*
+		 * a bitmap of size max_credits
+		 */
+		struct bitmap *credits_bitmap;
 		uint32_t max_trans;
 		uint32_t max_read;
 		uint32_t max_write;
-		struct bitmap *credits_bitmap;
 		bool compound_related_in_progress;
 	} smb2;
 };
diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
index 8a5d81f..cef0677 100644
--- a/source3/smbd/smb2_server.c
+++ b/source3/smbd/smb2_server.c
@@ -114,10 +114,11 @@ static NTSTATUS smbd_initialize_smb2(struct smbd_server_connection *sconn)
 	sconn->smb2.sessions.limit = 0x0000FFFE;
 	sconn->smb2.sessions.list = NULL;
 	sconn->smb2.seqnum_low = 0;
-	sconn->smb2.credits_granted = 0;
+	sconn->smb2.seqnum_range = 1;
+	sconn->smb2.credits_granted = 1;
 	sconn->smb2.max_credits = lp_smb2_max_credits();
 	sconn->smb2.credits_bitmap = bitmap_talloc(sconn,
-			DEFAULT_SMB2_MAX_CREDIT_BITMAP_FACTOR*sconn->smb2.max_credits);
+						   sconn->smb2.max_credits);
 	if (sconn->smb2.credits_bitmap == NULL) {
 		return NT_STATUS_NO_MEMORY;
 	}
@@ -298,67 +299,116 @@ static NTSTATUS smbd_smb2_request_create(struct smbd_server_connection *sconn,
 	return NT_STATUS_OK;
 }
 
-static bool smb2_validate_message_id(struct smbd_server_connection *sconn,
-				const uint8_t *inhdr)
+static bool smb2_validate_sequence_number(struct smbd_server_connection *sconn,
+					  uint64_t message_id, uint64_t seq_id)
 {
-	uint64_t message_id = BVAL(inhdr, SMB2_HDR_MESSAGE_ID);
 	struct bitmap *credits_bm = sconn->smb2.credits_bitmap;
-	uint16_t opcode = IVAL(inhdr, SMB2_HDR_OPCODE);
-	unsigned int bitmap_offset;
+	unsigned int offset;
 
-	if (opcode == SMB2_OP_CANCEL) {
-		/* SMB2_CANCEL requests by definition resend messageids. */
-		return true;
+	if (seq_id < sconn->smb2.seqnum_low) {
+		DEBUG(0,("smb2_validate_sequence_number: bad message_id "
+			"%llu (sequence id %llu) "
+			"(granted = %u, low = %llu, range = %u)\n",
+			(unsigned long long)message_id,
+			(unsigned long long)seq_id,
+			(unsigned int)sconn->smb2.credits_granted,
+			(unsigned long long)sconn->smb2.seqnum_low,
+			(unsigned int)sconn->smb2.seqnum_range));
+		return false;
 	}
 
-	if (message_id < sconn->smb2.seqnum_low ||
-			message_id > (sconn->smb2.seqnum_low +
-			(sconn->smb2.max_credits * DEFAULT_SMB2_MAX_CREDIT_BITMAP_FACTOR))) {
-		DEBUG(0,("smb2_validate_message_id: bad message_id "
-			"%llu (low = %llu, max = %lu)\n",
+	if (seq_id >= sconn->smb2.seqnum_low + sconn->smb2.seqnum_range) {
+		DEBUG(0,("smb2_validate_sequence_number: bad message_id "
+			"%llu (sequence id %llu) "
+			"(granted = %u, low = %llu, range = %u)\n",
 			(unsigned long long)message_id,
+			(unsigned long long)seq_id,
+			(unsigned int)sconn->smb2.credits_granted,
 			(unsigned long long)sconn->smb2.seqnum_low,
-			(unsigned long)sconn->smb2.max_credits ));
+			(unsigned int)sconn->smb2.seqnum_range));
 		return false;
 	}
 
-	if (sconn->smb2.credits_granted == 0) {
-		DEBUG(0,("smb2_validate_message_id: client used more "
-			 "credits than granted message_id (%llu)\n",
-			 (unsigned long long)message_id));
+	offset = seq_id % sconn->smb2.max_credits;
+
+	if (bitmap_query(credits_bm, offset)) {
+		DEBUG(0,("smb2_validate_sequence_number: duplicate message_id "
+			"%llu (sequence id %llu) "
+			"(granted = %u, low = %llu, range = %u) "
+			"(bm offset %u)\n",
+			(unsigned long long)message_id,
+			(unsigned long long)seq_id,
+			(unsigned int)sconn->smb2.credits_granted,
+			(unsigned long long)sconn->smb2.seqnum_low,
+			(unsigned int)sconn->smb2.seqnum_range,
+			offset));
 		return false;
 	}
 
-	/* client just used a credit. */
-	sconn->smb2.credits_granted -= 1;
+	/* Mark the message_ids as seen in the bitmap. */
+	bitmap_set(credits_bm, offset);
 
-	/* Mark the message_id as seen in the bitmap. */
-	bitmap_offset = (unsigned int)(message_id %
-			(uint64_t)(sconn->smb2.max_credits * DEFAULT_SMB2_MAX_CREDIT_BITMAP_FACTOR));
-	if (bitmap_query(credits_bm, bitmap_offset)) {
-		DEBUG(0,("smb2_validate_message_id: duplicate message_id "
-			"%llu (bm offset %u)\n",
-			(unsigned long long)message_id,
-			bitmap_offset));
+	if (seq_id != sconn->smb2.seqnum_low) {
+		return true;
+	}
+
+	/*
+	 * Move the window forward by all the message_id's
+	 * already seen.
+	 */
+	while (bitmap_query(credits_bm, offset)) {
+		DEBUG(10,("smb2_validate_sequence_number: clearing "
+			  "id %llu (position %u) from bitmap\n",
+			  (unsigned long long)(sconn->smb2.seqnum_low),
+			  offset));
+		bitmap_clear(credits_bm, offset);
+
+		sconn->smb2.seqnum_low += 1;
+		sconn->smb2.seqnum_range -= 1;
+		offset = sconn->smb2.seqnum_low % sconn->smb2.max_credits;
+	}
+
+	return true;
+}
+
+static bool smb2_validate_message_id(struct smbd_server_connection *sconn,
+				const uint8_t *inhdr)
+{
+	uint64_t message_id = BVAL(inhdr, SMB2_HDR_MESSAGE_ID);
+	uint16_t opcode = IVAL(inhdr, SMB2_HDR_OPCODE);
+	bool ok;
+
+	if (opcode == SMB2_OP_CANCEL) {
+		/* SMB2_CANCEL requests by definition resend messageids. */
+		return true;
+	}
+
+	DEBUG(11, ("smb2_validate_message_id: mid %llu, credits_granted %llu, "
+		   "seqnum low/range: %llu/%llu\n",
+		   (unsigned long long) message_id,
+		   (unsigned long long) sconn->smb2.credits_granted,
+		   (unsigned long long) sconn->smb2.seqnum_low,
+		   (unsigned long long) sconn->smb2.seqnum_range));
+
+	if (sconn->smb2.credits_granted < 1) {
+		DEBUG(0, ("smb2_validate_message_id: client used more "
+			  "credits than granted, mid %llu, credits_granted %llu, "
+			  "seqnum low/range: %llu/%llu\n",
+			  (unsigned long long) message_id,
+			  (unsigned long long) sconn->smb2.credits_granted,
+			  (unsigned long long) sconn->smb2.seqnum_low,
+			  (unsigned long long) sconn->smb2.seqnum_range));
 		return false;
 	}
-	bitmap_set(credits_bm, bitmap_offset);
 
-	if (message_id == sconn->smb2.seqnum_low + 1) {
-		/* Move the window forward by all the message_id's
-		   already seen. */
-		while (bitmap_query(credits_bm, bitmap_offset)) {
-			DEBUG(10,("smb2_validate_message_id: clearing "
-				"id %llu (position %u) from bitmap\n",
-				(unsigned long long)(sconn->smb2.seqnum_low + 1),
-				bitmap_offset ));
-			bitmap_clear(credits_bm, bitmap_offset);
-			sconn->smb2.seqnum_low += 1;
-			bitmap_offset = (bitmap_offset + 1) %
-				(sconn->smb2.max_credits * DEFAULT_SMB2_MAX_CREDIT_BITMAP_FACTOR);
-		}
+	ok = smb2_validate_sequence_number(sconn, message_id, message_id);
+	if (!ok) {
+		return false;
 	}
 
+	/* substract used credits */
+	sconn->smb2.credits_granted -= 1;
+
 	return true;
 }
 
@@ -453,10 +503,33 @@ static void smb2_set_operation_credit(struct smbd_server_connection *sconn,
 	uint8_t *outhdr = (uint8_t *)out_vector->iov_base;
 	uint16_t credits_requested;
 	uint32_t out_flags;
+	uint16_t cmd;
+	NTSTATUS out_status;
 	uint16_t credits_granted = 0;
+	uint64_t credits_possible;
+	uint16_t current_max_credits;
+
+	/*
+	 * first we grant only 1/16th of the max range.
+	 *
+	 * Windows also starts with the 1/16th and then grants
+	 * more later. I was only able to trigger higher
+	 * values, when using a verify high credit charge.
+	 *
+	 * TODO: scale up depending one load, free memory
+	 *       or other stuff.
+	 *       Maybe also on the relationship between number
+	 *       of requests and the used sequence number.
+	 *       Which means we would grant more credits
+	 *       for client which use multi credit requests.
+	 */
+	current_max_credits = sconn->smb2.max_credits / 16;
+	current_max_credits = MAX(current_max_credits, 1);
 
+	cmd = SVAL(inhdr, SMB2_HDR_OPCODE);
 	credits_requested = SVAL(inhdr, SMB2_HDR_CREDIT);
 	out_flags = IVAL(outhdr, SMB2_HDR_FLAGS);
+	out_status = NT_STATUS(IVAL(outhdr, SMB2_HDR_STATUS));
 
 	SMB_ASSERT(sconn->smb2.max_credits >= sconn->smb2.credits_granted);
 
@@ -466,48 +539,83 @@ static void smb2_set_operation_credit(struct smbd_server_connection *sconn,
 		 * response, we should not grant
 		 * credits on the final response.
 		 */
-		credits_requested = 0;
-	}
+		credits_granted = 0;
+	} else if (credits_requested > 0) {
+		uint16_t additional_max = 0;
+		uint16_t additional_credits = credits_requested - 1;
+
+		switch (cmd) {
+		case SMB2_OP_NEGPROT:
+			break;
+		case SMB2_OP_SESSSETUP:
+			/*
+			 * Windows 2012 RC1 starts to grant
+			 * additional credits
+			 * with a successful session setup
+			 */
+			if (NT_STATUS_IS_OK(out_status)) {
+				additional_max = 32;
+			}
+			break;
+		default:
+			/*
+			 * We match windows and only grant additional credits
+			 * in chunks of 32.
+			 */
+			additional_max = 32;
+			break;
+		}
 
-	if (credits_requested) {
-		uint16_t modified_credits_requested;
-		uint32_t multiplier;
+		additional_credits = MIN(additional_credits, additional_max);
 
+		credits_granted = 1 + additional_credits;
+	} else if (sconn->smb2.credits_granted == 0) {
 		/*
-		 * Split up max_credits into 1/16ths, and then scale
-		 * the requested credits by how many 16ths have been
-		 * currently granted. Less than 1/16th == grant all
-		 * requested (100%), scale down as more have been
-		 * granted. Never ask for less than 1 as the client
-		 * asked for at least 1. JRA.
+		 * Make sure the client has always at least one credit
 		 */
-
-		multiplier = 16 - (((sconn->smb2.credits_granted * 16) / sconn->smb2.max_credits) % 16);
-
-		modified_credits_requested = (multiplier * credits_requested) / 16;
-		if (modified_credits_requested == 0) {
-			modified_credits_requested = 1;
-		}
-
-		/* Remember what we gave out. */
-		credits_granted = MIN(modified_credits_requested,
-					(sconn->smb2.max_credits - sconn->smb2.credits_granted));
+		credits_granted = 1;
 	}
 
-	if (credits_granted == 0 && sconn->smb2.credits_granted == 0) {
-		/* First negprot packet, or ensure the client credits can
-		   never drop to zero. */
-		credits_granted = 1;
+	/*
+	 * sequence numbers should not wrap
+	 *
+	 * 1. calculate the possible credits until
+	 *    the sequence numbers start to wrap on 64-bit.
+	 *
+	 * 2. UINT64_MAX is used for Break Notifications.
+	 *
+	 * 2. truncate the possible credits to the maximum
+	 *    credits we want to grant to the client in total.
+	 *
+	 * 3. remove the range we'll already granted to the client
+	 *    this makes sure the client consumes the lowest sequence
+	 *    number, before we can grant additional credits.
+	 */
+	credits_possible = UINT64_MAX - sconn->smb2.seqnum_low;
+	if (credits_possible > 0) {
+		/* remove UINT64_MAX */
+		credits_possible -= 1;
 	}
+	credits_possible = MIN(credits_possible, current_max_credits);
+	credits_possible -= sconn->smb2.seqnum_range;
+
+	credits_granted = MIN(credits_granted, credits_possible);
 
 	SSVAL(outhdr, SMB2_HDR_CREDIT, credits_granted);
 	sconn->smb2.credits_granted += credits_granted;
+	sconn->smb2.seqnum_range += credits_granted;
 
 	DEBUG(10,("smb2_set_operation_credit: requested %u, "
-		"granted %u, total granted %u\n",
+		"granted %u, current possible/max %u/%u, "
+		"total granted/max/low/range %u/%u/%llu/%u\n",
 		(unsigned int)credits_requested,
 		(unsigned int)credits_granted,
-		(unsigned int)sconn->smb2.credits_granted ));
+		(unsigned int)credits_possible,
+		(unsigned int)current_max_credits,
+		(unsigned int)sconn->smb2.credits_granted,
+		(unsigned int)sconn->smb2.max_credits,
+		(unsigned long long)sconn->smb2.seqnum_low,
+		(unsigned int)sconn->smb2.seqnum_range));
 }
 
 static void smb2_calculate_credits(const struct smbd_smb2_request *inreq,
@@ -2522,6 +2630,12 @@ void smbd_smb2_first_negprot(struct smbd_server_connection *sconn,
 		return;
 	}
 
+	status = smbd_smb2_request_validate(req);
+	if (!NT_STATUS_IS_OK(status)) {
+		smbd_server_connection_terminate(sconn, nt_errstr(status));
+		return;
+	}
+
 	status = smbd_smb2_request_setup_out(req);
 	if (!NT_STATUS_IS_OK(status)) {
 		smbd_server_connection_terminate(sconn, nt_errstr(status));


-- 
Samba Shared Repository


More information about the samba-cvs mailing list