[Ipmitool-cvs] ipmitool/src/plugins/lan lan.c,1.66,1.67

[Jean-Michel A. <au...@us...>]

Update of /cvsroot/ipmitool/ipmitool/src/plugins/lan
In directory vz-cvs-4.sog:/tmp/cvs-serv28213/src/plugins/lan

Modified Files:
	lan.c 
Log Message:
- Bug in the existing code where it keeps on adding same command/seq pair 
- Add timeout / retry to LAN
-- Thanks to Harshad Parbhu --


Index: lan.c
===================================================================
RCS file: /cvsroot/ipmitool/ipmitool/src/plugins/lan/lan.c,v
retrieving revision 1.66
retrieving revision 1.67
diff -C2 -d -r1.66 -r1.67
*** lan.c	16 Apr 2010 13:49:30 -0000	1.66
--- lan.c	22 Nov 2011 13:43:38 -0000	1.67
***************
*** 692,699 ****
  		curr_seq = 0;
  
! 	entry = ipmi_req_add_entry(intf, req, curr_seq);
! 	if (entry == NULL)
! 		return NULL;
! 
  	len = req->msg.data_len + 29;
  	if (s->active && s->authtype)
--- 692,720 ----
  		curr_seq = 0;
  
! 	// Bug in the existing code where it keeps on adding same command/seq pair 
! 	// in the lookup entry list.
! 	// Check if we have cmd,seq pair already in our list. As we are not changing 
! 	// the seq number we have to re-use the node which has existing
! 	// command and sequence number. If we add then we will have redundant node with
! 	// same cmd,seq pair
! 	entry = ipmi_req_lookup_entry(curr_seq, req->msg.cmd);
! 	if (entry)
! 	{
! 		// This indicates that we have already same command and seq in list
! 		// No need to add once again and we will re-use the existing node.
! 		// Only thing we have to do is clear the msg_data as we create
! 		// a new one below in the code for it.
! 		if (entry->msg_data)
! 			free(entry->msg_data);
! 	}
! 	else
! 	{
! 		// We dont have this request in the list so we can add it 
! 		// to the list
! 		entry = ipmi_req_add_entry(intf, req, curr_seq);
! 		if (entry == NULL)
! 			return NULL;
! 	}
!  
  	len = req->msg.data_len + 29;
  	if (s->active && s->authtype)
***************
*** 915,918 ****
--- 936,963 ----
  	}
  
+ 	// We need to cleanup the existing entries from the list. Because if we 
+ 	// keep it and then when we send the new command and if the response is for
+ 	// old command it still matches it and then returns success.
+ 	// This is the corner case where the remote controller responds very slowly.
+ 	//
+ 	// Example: We have to send command 23 and 2d.
+ 	// If we send command,seq as 23,10 and if we dont get any response it will 
+ 	// retry 4 times with 23,10 and then come out here and indicate that there is no
+ 	// reponse from the remote controller and will send the next command for 
+ 	// ie 2d,11. And if the BMC is slow to respond and returns 23,10 then it 
+ 	// will match it in the list and will take response of command 23 as response 
+ 	// for command 2d and return success. So ideally when retries are done and 
+ 	// are out of this function we should be clearing the list to be safe so that
+ 	// we dont match the old response with new request.
+ 	//          [23, 10] --> BMC
+ 	//          [23, 10] --> BMC
+ 	//          [23, 10] --> BMC
+ 	//          [23, 10] --> BMC
+ 	//          [2D, 11] --> BMC
+ 	//                   <-- [23, 10]
+ 	//  here if we maintain 23,10 in the list then it will get matched and consider
+ 	//  23 response as response for 2D.   
+ 	ipmi_req_clear_entries();
+  
  	return rsp;
  }