[Ipmitool-devel] [ ipmitool-Patches-3611254 ] OEM handle for Intel 82751 in SPT mode

Discussion:

Dmitry Bazhenov

2013-04-18 06:00:16 UTC

Hello, all,

The attached patch adds a OEM handle and workaround code for Intel
i82751 MAC working in the super pass-though mode which has known
deviations in its RMCP+ implementation. With the OEM handle active,
IPMITool can successfully interact with this Intel 82751-based boards.

Please, review.

Regards,
Dmitry

Zdenek Styblik

2013-04-18 11:03:45 UTC

Permalink

Post by Dmitry Bazhenov
Hello, all,
The attached patch adds a OEM handle and workaround code for Intel i82751
MAC working in the super pass-though mode which has known deviations in its
RMCP+ implementation. With the OEM handle active, IPMITool can successfully
interact with this Intel 82751-based boards.
Please, review.
Regards,
Dmitry

Hi,

it's hard to say, but it seems to me the following diffs have wrong indentation:
@@ -1568,6 +1568,8 @@ ipmi_sol_red_pill(struct ipmi_intf * int
FD_SET(0, &read_fds);
FD_SET(intf->fd, &read_fds);

+ if (!ipmi_oem_active(intf,"i82571spt"))
+ {
/* Send periodic keepalive packet */
if(_use_sol_for_keepalive == 0)
{
@@ -1604,7 +1606,7 @@ ipmi_sol_red_pill(struct ipmi_intf * int
/* if the keep Alive is successful reset retries to zero */
retrySol = 0;
}
-
+ } /* !oem="i82571spt" */
/* Wait up to half a second */
tv.tv_sec = 0;
tv.tv_usec = 500000;
~~~

And since the following is getting changed:
@@ -460,7 +469,7 @@ int lanplus_generate_rakp3_authcode(uint
* returns 0 on success
* 1 on failure
*/
-int lanplus_generate_sik(struct ipmi_session * session)
+int lanplus_generate_sik(struct ipmi_session * session, struct
ipmi_intf * intf)
{
uint8_t * input_buffer;
int input_buffer_length, i;
~~~

change it to(just an example what I mean and can be done by person
doing commit):
@@ -460,7 +469,8 @@ int lanplus_generate_rakp3_authcode(uint
* returns 0 on success
* 1 on failure
*/
-int lanplus_generate_sik(struct ipmi_session * session)
+int
+lanplus_generate_sik(struct ipmi_session * session, struct ipmi_intf * intf)
{
uint8_t * input_buffer;
int input_buffer_length, i;
~~~

Thanks,
Z.

Dmitry Bazhenov

2013-04-19 06:56:39 UTC

Permalink

Zdenek,

I corrected the indentation and formatting. Please, review.

Regards,
Dmitry

Post by Zdenek Styblik

Hi,
@@ -1568,6 +1568,8 @@ ipmi_sol_red_pill(struct ipmi_intf * int
FD_SET(0, &read_fds);
FD_SET(intf->fd, &read_fds);
+ if (!ipmi_oem_active(intf,"i82571spt"))
+ {
/* Send periodic keepalive packet */
if(_use_sol_for_keepalive == 0)
{
@@ -1604,7 +1606,7 @@ ipmi_sol_red_pill(struct ipmi_intf * int
/* if the keep Alive is successful reset retries to zero */
retrySol = 0;
}
-
+ } /* !oem="i82571spt" */
/* Wait up to half a second */
tv.tv_sec = 0;
tv.tv_usec = 500000;
~~~
@@ -460,7 +469,7 @@ int lanplus_generate_rakp3_authcode(uint
* returns 0 on success
* 1 on failure
*/
-int lanplus_generate_sik(struct ipmi_session * session)
+int lanplus_generate_sik(struct ipmi_session * session, struct
ipmi_intf * intf)
{
uint8_t * input_buffer;
int input_buffer_length, i;
~~~
change it to(just an example what I mean and can be done by person
@@ -460,7 +469,8 @@ int lanplus_generate_rakp3_authcode(uint
* returns 0 on success
* 1 on failure
*/
-int lanplus_generate_sik(struct ipmi_session * session)
+int
+lanplus_generate_sik(struct ipmi_session * session, struct ipmi_intf * intf)
{
uint8_t * input_buffer;
int input_buffer_length, i;
~~~
Thanks,
Z.

Dmitry Bazhenov

2013-04-23 04:35:12 UTC

Permalink

Hello, Zdenek,

Are there any other comments to the patch or I can treat it is accepted?

Regards,
Dmitry

Post by Dmitry Bazhenov
Zdenek,
I corrected the indentation and formatting. Please, review.
Regards,
Dmitry

On Thu, Apr 18, 2013 at 8:00 AM, Dmitry Bazhenov

------------------------------------------------------------------------------
Precog is a next-generation analytics platform capable of advanced
analytics on semi-structured data. The platform includes APIs for building
apps and a phenomenal toolset for data science. Developers can use
our toolset for easy data analysis & visualization. Get a free account!
http://www2.precog.com/precogplatform/slashdotnewsletter
_______________________________________________
Ipmitool-devel mailing list
https://lists.sourceforge.net/lists/listinfo/ipmitool-devel

Zdenek Styblik

2013-04-23 07:38:35 UTC

Permalink

Post by Dmitry Bazhenov
Hello, Zdenek,
Are there any other comments to the patch or I can treat it is accepted?
Regards,
Dmitry

Dmitry,

no more comments about this one. However, you might want to wait till
some pro swings around and gives you his blessing, or whatever.
I have a comment to whoever is going to commit submitted diff. It
would be worth to commit formatting changes to 'lan_crypt.c'
separately.
Dmitry, don't re-do patch again. I'd say it's my fault and we
misunderstood each other there. It just should be committed as a
separate commit and that's all. And to separate it is not that much
work for whoever is going to do it.

Regards,
Z.

Post by Dmitry Bazhenov

Post by Dmitry Bazhenov
Zdenek,
I corrected the indentation and formatting. Please, review.
Regards,
Dmitry

On Thu, Apr 18, 2013 at 8:00 AM, Dmitry Bazhenov

------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_apr
_______________________________________________
Ipmitool-devel mailing list
https://lists.sourceforge.net/lists/listinfo/ipmitool-devel

Zdenek Styblik

2013-05-27 11:25:33 UTC

Permalink

On Tue, Apr 23, 2013 at 9:38 AM, Zdenek Styblik

Post by Zdenek Styblik

Post by Dmitry Bazhenov
Hello, Zdenek,
Are there any other comments to the patch or I can treat it is accepted?
Regards,
Dmitry

Dmitry,
no more comments about this one. However, you might want to wait till
some pro swings around and gives you his blessing, or whatever.
I have a comment to whoever is going to commit submitted diff. It
would be worth to commit formatting changes to 'lan_crypt.c'
separately.
Dmitry, don't re-do patch again. I'd say it's my fault and we
misunderstood each other there. It just should be committed as a
separate commit and that's all. And to separate it is not that much
work for whoever is going to do it.
Regards,
Z.

Hello Dmitry,

I'd like to (finally) commit this. I believe it's still relevant, right?
There is only one problem with applying your patch. File
'src/plugins/lanplus/lanplus.c ' got changed[1]. I've adjusted patch
by hand as can be seen below, but I want your confirmation this
adjustment is correct.
~~~
@@ -3462,11 +3463,11 @@

lprintf(LOG_DEBUG, "IPMIv2 / RMCP+ SESSION OPENED SUCCESSFULLY\n");

- rc = ipmi_set_session_privlvl_cmd(intf);
-
- if (rc < 0)
- goto fail;
-
+ if (!ipmi_oem_active(intf, "i82571spt")) {
+ rc = ipmi_set_session_privlvl_cmd(intf);
+ if (rc < 0)
+ goto fail;
+ }
intf->manufacturer_id = ipmi_get_oem(intf);
bridgePossible = 1;
~~~

Once confirmed, I'll commit it and serial interface right after.

Thanks,
Z.

[1] http://ipmitool.cvs.sourceforge.net/viewvc/ipmitool/ipmitool/src/plugins/lanplus/lanplus.c?r1=1.73&r2=1.74&

Post by Zdenek Styblik

Post by Dmitry Bazhenov

Post by Dmitry Bazhenov
Zdenek,
I corrected the indentation and formatting. Please, review.
Regards,
Dmitry

On Thu, Apr 18, 2013 at 8:00 AM, Dmitry Bazhenov

Dmitry Bazhenov

2013-05-27 11:53:59 UTC

Permalink

Hello, Zdenek,

I confirm the patches are relevant.
I confirm the change is correct.

Regards,
Dmitry

Post by Zdenek Styblik
On Tue, Apr 23, 2013 at 9:38 AM, Zdenek Styblik

Post by Zdenek Styblik

Post by Dmitry Bazhenov
Hello, Zdenek,
Are there any other comments to the patch or I can treat it is accepted?
Regards,
Dmitry

Dmitry,
no more comments about this one. However, you might want to wait till
some pro swings around and gives you his blessing, or whatever.
I have a comment to whoever is going to commit submitted diff. It
would be worth to commit formatting changes to 'lan_crypt.c'
separately.
Dmitry, don't re-do patch again. I'd say it's my fault and we
misunderstood each other there. It just should be committed as a
separate commit and that's all. And to separate it is not that much
work for whoever is going to do it.
Regards,
Z.

Hello Dmitry,
I'd like to (finally) commit this. I believe it's still relevant, right?
There is only one problem with applying your patch. File
'src/plugins/lanplus/lanplus.c ' got changed[1]. I've adjusted patch
by hand as can be seen below, but I want your confirmation this
adjustment is correct.
~~~
@@ -3462,11 +3463,11 @@
lprintf(LOG_DEBUG, "IPMIv2 / RMCP+ SESSION OPENED SUCCESSFULLY\n");
- rc = ipmi_set_session_privlvl_cmd(intf);
-
- if (rc < 0)
- goto fail;
-
+ if (!ipmi_oem_active(intf, "i82571spt")) {
+ rc = ipmi_set_session_privlvl_cmd(intf);
+ if (rc < 0)
+ goto fail;
+ }
intf->manufacturer_id = ipmi_get_oem(intf);
bridgePossible = 1;
~~~
Once confirmed, I'll commit it and serial interface right after.
Thanks,
Z.
[1] http://ipmitool.cvs.sourceforge.net/viewvc/ipmitool/ipmitool/src/plugins/lanplus/lanplus.c?r1=1.73&r2=1.74&

Post by Zdenek Styblik

Post by Dmitry Bazhenov

Post by Dmitry Bazhenov
Zdenek,
I corrected the indentation and formatting. Please, review.
Regards,
Dmitry

On Thu, Apr 18, 2013 at 8:00 AM, Dmitry Bazhenov