Bug 6864 - Problem in Solaris of FIONREAD
Summary: Problem in Solaris of FIONREAD
Status: NEW
Alias: None
Product: Samba 3.4
Classification: Unclassified
Component: Client Tools (show other bugs)
Version: 3.4.3
Hardware: Sparc Solaris
: P3 major
Target Milestone: ---
Assignee: Volker Lendecke
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-11-01 02:08 UTC by ryu
Modified: 2009-11-01 05:06 UTC (History)
1 user (show)

See Also:


Attachments
patch (4.38 KB, patch)
2009-11-01 02:12 UTC, ryu
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description ryu 2009-11-01 02:08:59 UTC
please see patch

diff -rc source3-org/lib/packet.c source3/lib/packet.c
*** source3-org/lib/packet.c	Thu Oct 29 16:47:16 2009
--- source3/lib/packet.c	Sun Nov  1 16:51:11 2009
***************
*** 58,63 ****
--- 58,90 ----
  	size_t new_size;
  	uint8 *in;
  
+ #if defined(__sun__) 
+ 	/*  see libsmb/async_smb.c */
+ #define FIONREAD_RETRY 10
+ 	{
+ 	  int retry = 0;
+ 	  while(1) {
+ 	    res = ioctl(ctx->fd, FIONREAD, &available);
+ 	    
+ 	    if (res == -1) {
+ 	      DEBUG(10, ("ioctl(FIONREAD) failed: %s\n", strerror(errno)));
+ 	      return map_nt_error_from_unix(errno);
+ 	    }
+ 	    
+ 	    if (available == 0) {
+ 	      if(retry > FIONREAD_RETRY) {
+ 		return NT_STATUS_END_OF_FILE;
+ 	      } else {
+ 		retry++;
+ 		DEBUG(1, ("ioctl(FIONREAD) retry: %d\n", retry));
+ 		usleep(100000); 
+ 	      }
+ 	    } else {
+ 	      break;
+ 	    }
+ 	  }
+ 	}
+ #else
  	res = ioctl(ctx->fd, FIONREAD, &available);
  
  	if (res == -1) {
***************
*** 70,76 ****
  	if (available == 0) {
  		return NT_STATUS_END_OF_FILE;
  	}
! 
  	new_size = ctx->in.length + available;
  
  	if (new_size < ctx->in.length) {
--- 97,103 ----
  	if (available == 0) {
  		return NT_STATUS_END_OF_FILE;
  	}
! #endif
  	new_size = ctx->in.length + available;
  
  	if (new_size < ctx->in.length) {
diff -rc source3-org/libsmb/async_smb.c source3/libsmb/async_smb.c
*** source3-org/libsmb/async_smb.c	Thu Oct 29 16:47:16 2009
--- source3/libsmb/async_smb.c	Sun Nov  1 16:51:11 2009
***************
*** 1048,1053 ****
--- 1048,1097 ----
  		size_t old_size, new_size;
  		char *tmp;
  
+ 
+ #if defined(__sun__) 
+ 		/*  man streamio
+ 		    I_NREAD
+                        Counts the number of data  bytes  in  data
+                        blocks  in the first message on the STREAM
+                        head read queue, and places this value  in
+                        the location pointed to by arg. The return
+                        value for the command  is  the  number  of
+                        messages  on  the  STREAM head read queue.
+                        For example, if zero is returned  in  arg,
+                        but the ioctl return value is greater than
+                        zero, this indicates  that  a  zero-length
+                        message  is next on the queue. On failure,
+                        errno is set to the following value:
+ 		*/
+ #define FIONREAD_RETRY 10
+ 		{
+ 		  int retry = 0;
+ 		  while(1) {
+ 		    res = ioctl(cli->fd, FIONREAD, &available);
+ 		    if (res == -1) {
+ 		      DEBUG(10, ("ioctl(FIONREAD) failed: %s\n",
+ 				 strerror(errno)));
+ 		      status = map_nt_error_from_unix(errno);
+ 		      goto sock_error;
+ 		    }
+ 		    
+ 		    if (available == 0) {
+ 		      /* EOF */
+ 		      if(retry > FIONREAD_RETRY) {
+ 			status = NT_STATUS_END_OF_FILE;
+ 			goto sock_error;
+ 		      } else {
+ 			retry++;
+ 			DEBUG(1, ("ioctl(FIONREAD) retry: %d\n", retry));
+ 			usleep(100000); 
+ 		      }
+ 		    } else {
+ 		      break;
+ 		    }
+ 		  }
+ 		}
+ #else
  		res = ioctl(cli->fd, FIONREAD, &available);
  		if (res == -1) {
  			DEBUG(10, ("ioctl(FIONREAD) failed: %s\n",
***************
*** 1061,1066 ****
--- 1105,1111 ----
  			status = NT_STATUS_END_OF_FILE;
  			goto sock_error;
  		}
+ #endif
  
  		old_size = talloc_get_size(cli->evt_inbuf);
  		new_size = old_size + available;
diff -rc source3-org/smbd/notify_inotify.c source3/smbd/notify_inotify.c
*** source3-org/smbd/notify_inotify.c	Thu Oct 29 16:47:16 2009
--- source3/smbd/notify_inotify.c	Sun Nov  1 16:51:11 2009
***************
*** 239,249 ****
--- 239,275 ----
  	  filenames, and thus can't know how much to allocate
  	  otherwise
  	*/
+ #if defined(__sun__) 
+ 	/*  see libsmb/async_smb.c */
+ #define FIONREAD_RETRY 2
+ 	{
+ 	  int retry = 0;
+ 	  while(1) {
+ 	    if (ioctl(in->fd, FIONREAD, &bufsize) != 0) {
+ 	      DEBUG(0,("No data on inotify fd?!\n"));
+ 	      return;
+ 	    }
+ 	    if(bufsize == 0) {
+ 	      if(retry > FIONREAD_RETRY) {
+ 		DEBUG(0,("No data on inotify fd?!\n"));
+ 		return;
+ 	      } else {
+ 		retry++;
+ 		DEBUG(1, ("ioctl(FIONREAD) retry: %d\n", retry));
+ 		usleep(100000); 
+ 	      }
+ 	    } else {
+ 	      break;
+ 	    }
+ 	  }
+ 	}
+ #else
  	if (ioctl(in->fd, FIONREAD, &bufsize) != 0 || 
  	    bufsize == 0) {
  		DEBUG(0,("No data on inotify fd?!\n"));
  		return;
  	}
+ #endif
  
  	e0 = e = (struct inotify_event *)TALLOC_SIZE(in, bufsize);
  	if (e == NULL) return;
Comment 1 ryu 2009-11-01 02:12:28 UTC
Created attachment 4911 [details]
patch

sorry the patch was written in Description thinking that it was not possible to append it.
Comment 2 Volker Lendecke 2009-11-01 02:31:05 UTC
Thanks.

Question: What phenomenon are you seeing that made you create this patch? How can a zero-sized message show up in a TCP connection?

I see and I think I understand the patch. From the man page that you quote I don't see however that you have to retry twice after a timeout. The condition that a retry would be appropriate at would be

((available == 0) && (res > 0))

and then we might have to retry any number of times.

Next, instead of patching in that piece of code at three places, could you refactor that into a routine of its own?

Thanks,

Volker
Comment 3 ryu 2009-11-01 05:06:19 UTC
When a large amount of data was taken with smbtar, I encountered this phenomenon. 
The error code is NT_STATUS_END_OF_FILE. 
The generated timing is not constant. 

In Solaris, this patch was made because there was a case where FIONREAD(I_NREAD) returns by 0.