[Ipmitool-devel] [patches:#89] [BMR #81324] HPM.2 long message support

Post by Dmitry Bazhenov
Hello, Zdenek,
The attached patch adds basic long message support for PICMG-based systems
according to the HPM.2 specification.
The patch introduces APIs for setting inbound and outbound messages sizes
per selected interface.
These APIs are used in LAN and LAN+ interfaces to set autonomously detected
inbound and outbound message sizes.
The newly introduced APIs also replace the existing message size detection
code in several ipmitool commands in order to leverage the advantages of
long message support (HPM.1 upgrade, SDR acquring, FRU inventory read and
write).
The patch also moves the Kontron-specific long message support under a OEM
option.
Please, review. Unless, there are comments to address, I would like this
patch to be submitted into the source tree.
Regards,
Dmitry

Hello Dmitry,

I'll give it a look within a week. As usual, I can comment only as far
as general programming goes.

Take care,
Z.

Zdenek Styblik

2014-02-13 19:14:27 UTC

On Tue, Feb 11, 2014 at 9:00 AM, Zdenek Styblik
<***@gmail.com> wrote:
[...]

Dmitry,

my comments from pass#1 reading are as follows.

* Formatting - no worries about this one at this time ;)
* name 'ipmi_hpm2' - since it's not a CLI module, I think it would be
more fitting to name it 'hpm2' or whatever, but not 'ipmi_hpm2'. As
far as I've noticed, CLI modules are prefixed by 'ipmi_' whether
"support" modules aren't. I think it would be wise to keep this
convention. Or are there any plans 'ipmi_hpm2' becoming CLI module as
well? Although, it can be renamed any time in the future. For, as long
as it isn't CLI module, ...
* include/ipmitool/ipmi_hpm2.h

~~~
+extern int hpm2_get_capabilities(struct ipmi_intf * intf,
+ struct hpm2_lan_attach_capabilities * caps);
+extern int hpm2_get_lan_channel_capabilities(struct ipmi_intf * intf,
+ uint8_t hpm2_lan_params_start,
+ struct hpm2_lan_channel_capabilities * caps);
+extern int hpm2_detect_max_payload_size(struct ipmi_intf * intf);
~~~

Extern? I mean, extern? I haven't tried to compile without it, but it
just seems odd to use 'extern' here.

* lib/ipmi_hpm2.c

~~~
+ if (rsp) {
+ if (rsp->ccode == 0xC1) {
+ lprintf(LOG_DEBUG,"IPM Controller is not HPM.2 compatible");
+ return rsp->ccode;
+ } else if (rsp->ccode) {
+ lprintf(LOG_NOTICE,"Get HPM.x Capabilities request failed,"
+ " compcode = %x\n", rsp->ccode);
+ return rsp->ccode;
+ }
+ } else {
+ lprintf(LOG_NOTICE,"Error sending request\n");
+ return -1;
+ }
~~~
->
~~~
if (rsp == NULL) {
lprintf(LOG_NOTICE,"Error sending request\n");
return -1;
}
+ if (rsp->ccode == 0xC1) {
+ lprintf(LOG_DEBUG,"IPM Controller is not HPM.2 compatible");
+ return rsp->ccode;
+ } else if (rsp->ccode) {
+ lprintf(LOG_NOTICE,"Get HPM.x Capabilities request failed,"
+ " compcode = %x\n", rsp->ccode);
+ return rsp->ccode;
+ }
[ continue with the code in function
~~~

~~~
if (caps->lan_channel_mask) {
[...]
}
return 0;
~~~
->
~~~
if (!caps->lan_channel_mask) {
return 0;
}
[ continue with code from if() block here ]
~~~

This repeats couple times through the file in question.

* lib/ipmi_hpmfwupg.c

~~~
+ uploadCmd.req = malloc(ipmi_intf_get_max_request_data_size(intf));
~~~

This doesn't look like a good idea. You just shouldn't do things like this.

* src/plugins/lan/lan.c and src/plugins/lanplus/lanplus.c
~~~
+ /* automatically detect interface request and response sizes */
+ hpm2_detect_max_payload_size(intf);
+
~~~

Aren't you interested in errors?

~~~
+static void ipmi_lan_set_max_rq_data_size(struct ipmi_intf * intf,
uint16_t size);
+static void ipmi_lan_set_max_rp_data_size(struct ipmi_intf * intf,
uint16_t size);
~~~

I don't believe 'static' is required, is it?

* src/plugins/open/open.c
~~~
+#define IPMI_OPENIPMI_DEFAULT_PAYLOAD_SIZE 37
~~~

I hope this doesn't break anything :)

* Now, my biggest concern - data size calculation. Have you considered
the fact given/supplied size could be so small you will underflow
uint? I believe this should be handled. You should check whether
addition or subtraction doesn't cause overflow/underflow. At least in
my opinion.

So much for the first quick reading through.

Z.

Dmitry Bazhenov

2014-02-18 10:07:52 UTC

Hello, Zdenek,

Please, see my comments below.

Post by Zdenek Styblik
On Tue, Feb 11, 2014 at 9:00 AM, Zdenek Styblik
[...]
Dmitry,
my comments from pass#1 reading are as follows.
* Formatting - no worries about this one at this time ;)
* name 'ipmi_hpm2' - since it's not a CLI module, I think it would be
more fitting to name it 'hpm2' or whatever, but not 'ipmi_hpm2'. As
far as I've noticed, CLI modules are prefixed by 'ipmi_' whether
"support" modules aren't. I think it would be wise to keep this
convention. Or are there any plans 'ipmi_hpm2' becoming CLI module as
well? Although, it can be renamed any time in the future. For, as long
as it isn't CLI module, ...

Currently, only utility functions. In the future there will be some
commands. I'll rename the files.

Post by Zdenek Styblik
* include/ipmitool/ipmi_hpm2.h
~~~
+extern int hpm2_get_capabilities(struct ipmi_intf * intf,
+ struct hpm2_lan_attach_capabilities * caps);
+extern int hpm2_get_lan_channel_capabilities(struct ipmi_intf * intf,
+ uint8_t hpm2_lan_params_start,
+ struct hpm2_lan_channel_capabilities * caps);
+extern int hpm2_detect_max_payload_size(struct ipmi_intf * intf);
~~~
Extern? I mean, extern? I haven't tried to compile without it, but it
just seems odd to use 'extern' here.

"Extern" is the default storage class for functions in C. In this
example the extern linkage is just stated explicitly.

Post by Zdenek Styblik
* lib/ipmi_hpm2.c
~~~
+ if (rsp) {
+ if (rsp->ccode == 0xC1) {
+ lprintf(LOG_DEBUG,"IPM Controller is not HPM.2 compatible");
+ return rsp->ccode;
+ } else if (rsp->ccode) {
+ lprintf(LOG_NOTICE,"Get HPM.x Capabilities request failed,"
+ " compcode = %x\n", rsp->ccode);
+ return rsp->ccode;
+ }
+ } else {
+ lprintf(LOG_NOTICE,"Error sending request\n");
+ return -1;
+ }
~~~
->
~~~
if (rsp == NULL) {
lprintf(LOG_NOTICE,"Error sending request\n");
return -1;
}
+ if (rsp->ccode == 0xC1) {
+ lprintf(LOG_DEBUG,"IPM Controller is not HPM.2 compatible");
+ return rsp->ccode;
+ } else if (rsp->ccode) {
+ lprintf(LOG_NOTICE,"Get HPM.x Capabilities request failed,"
+ " compcode = %x\n", rsp->ccode);
+ return rsp->ccode;
+ }
[ continue with the code in function
~~~
~~~
if (caps->lan_channel_mask) {
[...]
}
return 0;
~~~
->
~~~
if (!caps->lan_channel_mask) {
return 0;
}
[ continue with code from if() block here ]
~~~
This repeats couple times through the file in question.
* lib/ipmi_hpmfwupg.c
~~~
+ uploadCmd.req = malloc(ipmi_intf_get_max_request_data_size(intf));
~~~
This doesn't look like a good idea. You just shouldn't do things like this.

I do not understand the argument. Please, elaborate more on this. The
code does what is required. What's wrong?

Post by Zdenek Styblik
* src/plugins/lan/lan.c and src/plugins/lanplus/lanplus.c
~~~
+ /* automatically detect interface request and response sizes */
+ hpm2_detect_max_payload_size(intf);
+
~~~
Aren't you interested in errors?

No at all. Boards may support HPM.2 and may not. If they are, the
function will set appropriate payload sizes.

Post by Zdenek Styblik
~~~
+static void ipmi_lan_set_max_rq_data_size(struct ipmi_intf * intf,
uint16_t size);
+static void ipmi_lan_set_max_rp_data_size(struct ipmi_intf * intf,
uint16_t size);
~~~
I don't believe 'static' is required, is it?

This functions are for the internal module use. Using static storage
type is logical here.

Post by Zdenek Styblik
* src/plugins/open/open.c
~~~
+#define IPMI_OPENIPMI_DEFAULT_PAYLOAD_SIZE 37
~~~
I hope this doesn't break anything :)

This surely does not break for requests, but can break for responses.
I'll fix it.

Post by Zdenek Styblik
* Now, my biggest concern - data size calculation. Have you considered
the fact given/supplied size could be so small you will underflow
uint? I believe this should be handled. You should check whether
addition or subtraction doesn't cause overflow/underflow. At least in
my opinion.

I'll add necessary checks where it is appropriate.

No more comments below.

Regards,
Dmitry

Post by Zdenek Styblik
So much for the first quick reading through.
Z.

Zdenek Styblik

2014-03-03 17:12:43 UTC

On Tue, Feb 18, 2014 at 11:07 AM, Dmitry Bazhenov
<***@pigeonpoint.com> wrote:
[...]

Post by Zdenek Styblik
* Formatting - no worries about this one at this time ;)
* name 'ipmi_hpm2' - since it's not a CLI module, I think it would be
more fitting to name it 'hpm2' or whatever, but not 'ipmi_hpm2'. As
far as I've noticed, CLI modules are prefixed by 'ipmi_' whether
"support" modules aren't. I think it would be wise to keep this
convention. Or are there any plans 'ipmi_hpm2' becoming CLI module as
well? Although, it can be renamed any time in the future. For, as long
as it isn't CLI module, ...

Currently, only utility functions. In the future there will be some
commands. I'll rename the files.

Good.

[...]

Post by Dmitry Bazhenov
"Extern" is the default storage class for functions in C. In this example
the extern linkage is just stated explicitly.

Ok.

Post by Zdenek Styblik
* lib/ipmi_hpm2.c
~~~
+ if (rsp) {
+ if (rsp->ccode == 0xC1) {
+ lprintf(LOG_DEBUG,"IPM Controller is not HPM.2 compatible");
+ return rsp->ccode;
+ } else if (rsp->ccode) {
+ lprintf(LOG_NOTICE,"Get HPM.x Capabilities request failed,"
+ " compcode = %x\n", rsp->ccode);
+ return rsp->ccode;
+ }
+ } else {
+ lprintf(LOG_NOTICE,"Error sending request\n");
+ return -1;
+ }
~~~
->
~~~
if (rsp == NULL) {
lprintf(LOG_NOTICE,"Error sending request\n");
return -1;
}
+ if (rsp->ccode == 0xC1) {
+ lprintf(LOG_DEBUG,"IPM Controller is not HPM.2 compatible");
+ return rsp->ccode;
+ } else if (rsp->ccode) {
+ lprintf(LOG_NOTICE,"Get HPM.x Capabilities request failed,"
+ " compcode = %x\n", rsp->ccode);
+ return rsp->ccode;
+ }
[ continue with the code in function
~~~
~~~
if (caps->lan_channel_mask) {
[...]
}
return 0;
~~~
->
~~~
if (!caps->lan_channel_mask) {
return 0;
}
[ continue with code from if() block here ]
~~~
This repeats couple times through the file in question.
* lib/ipmi_hpmfwupg.c
~~~
+ uploadCmd.req =
malloc(ipmi_intf_get_max_request_data_size(intf));
~~~
This doesn't look like a good idea. You just shouldn't do things like this.

I do not understand the argument. Please, elaborate more on this. The code
does what is required. What's wrong?

The argument is - it's just over-complicated. Please, return as early
possible and don't do if() for if().

~~~
if (rsp == NULL) {
/* error */
return (-1);
}
if (rsp->ccode > 0) {
/* different error */
return rsp->ccode;
}
~~~

Post by Zdenek Styblik
~~~
+ uploadCmd.req =
malloc(ipmi_intf_get_max_request_data_size(intf));
~~~

It should be split into something like:
~~~
int allocate_i = ipmi_intf_get_max_request_data_size(intf);
uploadCmd.req = malloc(allocate_i);
~~~

I mean, don't use return value directly as an argument for function.

No at all. Boards may support HPM.2 and may not. If they are, the function
will set appropriate payload sizes.

Ok.

This functions are for the internal module use. Using static storage type is
logical here.

Yeah, we had several discussions with friend of mine. It turned out we
should reconsider our view on 'static'. So, agreed. My bad.

Post by Zdenek Styblik
* src/plugins/open/open.c
~~~
+#define IPMI_OPENIPMI_DEFAULT_PAYLOAD_SIZE 37
~~~
I hope this doesn't break anything :)

This surely does not break for requests, but can break for responses. I'll
fix it.

Haha, not what I meant, but great :)

I'll add necessary checks where it is appropriate.

Good.

And I'm sorry for delay. It took some time to discuss 'static' thing,
but the fact is, I just kept this reply on back-burner. If this ever
happens again, just kick me to see whether I'm alive.

Thanks,
Z.

Dmitry Bazhenov

2014-03-03 17:29:47 UTC

Hello, Zdenek,

I addressed your comments. Please, review once more.

Regards,
Dmitry

Post by Zdenek Styblik
On Tue, Feb 18, 2014 at 11:07 AM, Dmitry Bazhenov
[...]

Currently, only utility functions. In the future there will be some
commands. I'll rename the files.

Good.

[...]

Post by Dmitry Bazhenov
"Extern" is the default storage class for functions in C. In this example
the extern linkage is just stated explicitly.

Ok.

Post by Zdenek Styblik
* lib/ipmi_hpm2.c
~~~
+ if (rsp) {
+ if (rsp->ccode == 0xC1) {
+ lprintf(LOG_DEBUG,"IPM Controller is not HPM.2 compatible");
+ return rsp->ccode;
+ } else if (rsp->ccode) {
+ lprintf(LOG_NOTICE,"Get HPM.x Capabilities request failed,"
+ " compcode = %x\n", rsp->ccode);
+ return rsp->ccode;
+ }
+ } else {
+ lprintf(LOG_NOTICE,"Error sending request\n");
+ return -1;
+ }
~~~
->
~~~
if (rsp == NULL) {
lprintf(LOG_NOTICE,"Error sending request\n");
return -1;
}
+ if (rsp->ccode == 0xC1) {
+ lprintf(LOG_DEBUG,"IPM Controller is not HPM.2 compatible");
+ return rsp->ccode;
+ } else if (rsp->ccode) {
+ lprintf(LOG_NOTICE,"Get HPM.x Capabilities request failed,"
+ " compcode = %x\n", rsp->ccode);
+ return rsp->ccode;
+ }
[ continue with the code in function
~~~
~~~
if (caps->lan_channel_mask) {
[...]
}
return 0;
~~~
->
~~~
if (!caps->lan_channel_mask) {
return 0;
}
[ continue with code from if() block here ]
~~~
This repeats couple times through the file in question.
* lib/ipmi_hpmfwupg.c
~~~
+ uploadCmd.req =
malloc(ipmi_intf_get_max_request_data_size(intf));
~~~
This doesn't look like a good idea. You just shouldn't do things like this.

I do not understand the argument. Please, elaborate more on this. The code
does what is required. What's wrong?

The argument is - it's just over-complicated. Please, return as early
possible and don't do if() for if().
~~~
if (rsp == NULL) {
/* error */
return (-1);
}
if (rsp->ccode > 0) {
/* different error */
return rsp->ccode;
}
~~~

Post by Zdenek Styblik
~~~
+ uploadCmd.req =
malloc(ipmi_intf_get_max_request_data_size(intf));
~~~

~~~
int allocate_i = ipmi_intf_get_max_request_data_size(intf);
uploadCmd.req = malloc(allocate_i);
~~~
I mean, don't use return value directly as an argument for function.

No at all. Boards may support HPM.2 and may not. If they are, the function
will set appropriate payload sizes.

Ok.

This functions are for the internal module use. Using static storage type is
logical here.

Yeah, we had several discussions with friend of mine. It turned out we
should reconsider our view on 'static'. So, agreed. My bad.

Post by Zdenek Styblik
* src/plugins/open/open.c
~~~
+#define IPMI_OPENIPMI_DEFAULT_PAYLOAD_SIZE 37
~~~
I hope this doesn't break anything :)

This surely does not break for requests, but can break for responses. I'll
fix it.

Haha, not what I meant, but great :)

I'll add necessary checks where it is appropriate.

Good.
And I'm sorry for delay. It took some time to discuss 'static' thing,
but the fact is, I just kept this reply on back-burner. If this ever
happens again, just kick me to see whether I'm alive.
Thanks,
Z.

Dmitry Bazhenov

2014-03-13 04:29:40 UTC

Hello, Zdenek,

Did you have a chance to make a review of the updated patch?

Regards,
Dmitry

Post by Dmitry Bazhenov
Hello, Zdenek,
I addressed your comments. Please, review once more.
Regards,
Dmitry

Post by Zdenek Styblik
On Tue, Feb 18, 2014 at 11:07 AM, Dmitry Bazhenov
[...]

Currently, only utility functions. In the future there will be some
commands. I'll rename the files.

Good.

[...]

Post by Dmitry Bazhenov
"Extern" is the default storage class for functions in C. In this example
the extern linkage is just stated explicitly.

Ok.

Post by Zdenek Styblik
* lib/ipmi_hpm2.c
~~~
+ if (rsp) {
+ if (rsp->ccode == 0xC1) {
+ lprintf(LOG_DEBUG,"IPM Controller is not HPM.2
compatible");
+ return rsp->ccode;
+ } else if (rsp->ccode) {
+ lprintf(LOG_NOTICE,"Get HPM.x Capabilities request failed,"
+ " compcode = %x\n", rsp->ccode);
+ return rsp->ccode;
+ }
+ } else {
+ lprintf(LOG_NOTICE,"Error sending request\n");
+ return -1;
+ }
~~~
->
~~~
if (rsp == NULL) {
lprintf(LOG_NOTICE,"Error sending request\n");
return -1;
}
+ if (rsp->ccode == 0xC1) {
+ lprintf(LOG_DEBUG,"IPM Controller is not HPM.2
compatible");
+ return rsp->ccode;
+ } else if (rsp->ccode) {
+ lprintf(LOG_NOTICE,"Get HPM.x Capabilities request failed,"
+ " compcode = %x\n", rsp->ccode);
+ return rsp->ccode;
+ }
[ continue with the code in function
~~~
~~~
if (caps->lan_channel_mask) {
[...]
}
return 0;
~~~
->
~~~
if (!caps->lan_channel_mask) {
return 0;
}
[ continue with code from if() block here ]
~~~
This repeats couple times through the file in question.
* lib/ipmi_hpmfwupg.c
~~~
+ uploadCmd.req =
malloc(ipmi_intf_get_max_request_data_size(intf));
~~~
This doesn't look like a good idea. You just shouldn't do things like this.

I do not understand the argument. Please, elaborate more on this. The code
does what is required. What's wrong?

The argument is - it's just over-complicated. Please, return as early
possible and don't do if() for if().
~~~
if (rsp == NULL) {
/* error */
return (-1);
}
if (rsp->ccode > 0) {
/* different error */
return rsp->ccode;
}
~~~

Post by Zdenek Styblik
~~~
+ uploadCmd.req =
malloc(ipmi_intf_get_max_request_data_size(intf));
~~~

~~~
int allocate_i = ipmi_intf_get_max_request_data_size(intf);
uploadCmd.req = malloc(allocate_i);
~~~
I mean, don't use return value directly as an argument for function.

No at all. Boards may support HPM.2 and may not. If they are, the function
will set appropriate payload sizes.

Ok.

This functions are for the internal module use. Using static storage type is
logical here.

Yeah, we had several discussions with friend of mine. It turned out we
should reconsider our view on 'static'. So, agreed. My bad.

Post by Zdenek Styblik
* src/plugins/open/open.c
~~~
+#define IPMI_OPENIPMI_DEFAULT_PAYLOAD_SIZE 37
~~~
I hope this doesn't break anything :)

This surely does not break for requests, but can break for
responses. I'll
fix it.

Haha, not what I meant, but great :)

I'll add necessary checks where it is appropriate.

Good.
And I'm sorry for delay. It took some time to discuss 'static' thing,
but the fact is, I just kept this reply on back-burner. If this ever
happens again, just kick me to see whether I'm alive.
Thanks,
Z.

Zdenek Styblik

2014-03-14 12:55:32 UTC

Post by Dmitry Bazhenov
Hello, Zdenek,
Did you have a chance to make a review of the updated patch?
Regards,
Dmitry

Hi Dmitry,

yes, although not as deeply as I'd like to.

I got a bit "scared" by solution applied to
ipmi_intf_get_max_request_data_size() and
ipmi_intf_get_max_response_data_size(). But then I've tried to compile
just this one function with all kinds of switches and compiler didn't
comply, so I guess it's ok.
I wonder, shouldn't be the same logic applied to
ipmi_lanp_set_max_rq_data_size() and ipmi_lanp_set_max_rp_data_size()
as well?

~~~
+ if (rsp) {
+ if (rsp->ccode == 0xC1) {
+ lprintf(LOG_DEBUG,"IPM Controller is not HPM.2 compatible");
+ return rsp->ccode;
+ } else if (rsp->ccode) {
+ lprintf(LOG_NOTICE,"Get HPM.x Capabilities request failed,"
+ " compcode = %x\n", rsp->ccode);
+ return rsp->ccode;
+ }
+ } else {
+ lprintf(LOG_NOTICE,"Error sending request\n");
+ return -1;
+ }
~~~

This seems to have been missed. :)

~~~
if (my_long_packet_set == 1) {
+ if (ipmi_oem_active(ipmi_main_intf, "kontron"))
+ {
/* Restore defaults */
ipmi_kontronoem_set_large_buffer( ipmi_main_intf, 0 );
+ }
~~~

This seems to be have strange indentation.

Other than these and code formatting, I guess it's ok.

Have a nice weekend,
Z.

Post by Dmitry Bazhenov
Hello, Zdenek,
I addressed your comments. Please, review once more.
Regards,
Dmitry

Post by Zdenek Styblik
On Tue, Feb 18, 2014 at 11:07 AM, Dmitry Bazhenov
[...]

Currently, only utility functions. In the future there will be some
commands. I'll rename the files.

Good.

[...]

Post by Dmitry Bazhenov
"Extern" is the default storage class for functions in C. In this example
the extern linkage is just stated explicitly.

Ok.

Post by Zdenek Styblik
* lib/ipmi_hpm2.c
~~~
+ if (rsp) {
+ if (rsp->ccode == 0xC1) {
+ lprintf(LOG_DEBUG,"IPM Controller is not HPM.2 compatible");
+ return rsp->ccode;
+ } else if (rsp->ccode) {
+ lprintf(LOG_NOTICE,"Get HPM.x Capabilities request failed,"
+ " compcode = %x\n", rsp->ccode);
+ return rsp->ccode;
+ }
+ } else {
+ lprintf(LOG_NOTICE,"Error sending request\n");
+ return -1;
+ }
~~~
->
~~~
if (rsp == NULL) {
lprintf(LOG_NOTICE,"Error sending request\n");
return -1;
}
+ if (rsp->ccode == 0xC1) {
+ lprintf(LOG_DEBUG,"IPM Controller is not HPM.2 compatible");
+ return rsp->ccode;
+ } else if (rsp->ccode) {
+ lprintf(LOG_NOTICE,"Get HPM.x Capabilities request failed,"
+ " compcode = %x\n", rsp->ccode);
+ return rsp->ccode;
+ }
[ continue with the code in function
~~~
~~~
if (caps->lan_channel_mask) {
[...]
}
return 0;
~~~
->
~~~
if (!caps->lan_channel_mask) {
return 0;
}
[ continue with code from if() block here ]
~~~
This repeats couple times through the file in question.
* lib/ipmi_hpmfwupg.c
~~~
+ uploadCmd.req =
malloc(ipmi_intf_get_max_request_data_size(intf));
~~~
This doesn't look like a good idea. You just shouldn't do things like this.

I do not understand the argument. Please, elaborate more on this. The code
does what is required. What's wrong?

The argument is - it's just over-complicated. Please, return as early
possible and don't do if() for if().
~~~
if (rsp == NULL) {
/* error */
return (-1);
}
if (rsp->ccode > 0) {
/* different error */
return rsp->ccode;
}
~~~

Post by Zdenek Styblik
~~~
+ uploadCmd.req =
malloc(ipmi_intf_get_max_request_data_size(intf));
~~~

~~~
int allocate_i = ipmi_intf_get_max_request_data_size(intf);
uploadCmd.req = malloc(allocate_i);
~~~
I mean, don't use return value directly as an argument for function.

No at all. Boards may support HPM.2 and may not. If they are, the function
will set appropriate payload sizes.

Ok.

This functions are for the internal module use. Using static storage type is
logical here.

Yeah, we had several discussions with friend of mine. It turned out we
should reconsider our view on 'static'. So, agreed. My bad.

Post by Zdenek Styblik
* src/plugins/open/open.c
~~~
+#define IPMI_OPENIPMI_DEFAULT_PAYLOAD_SIZE 37
~~~
I hope this doesn't break anything :)

This surely does not break for requests, but can break for responses. I'll
fix it.

Haha, not what I meant, but great :)

I'll add necessary checks where it is appropriate.

Good.
And I'm sorry for delay. It took some time to discuss 'static' thing,
but the fact is, I just kept this reply on back-burner. If this ever
happens again, just kick me to see whether I'm alive.
Thanks,
Z.

Dmitry Bazhenov

2014-03-19 07:33:50 UTC

Hello, Zdenek,

Please, see below marked with [DB].

Post by Dmitry Bazhenov
Hello, Zdenek,
Did you have a chance to make a review of the updated patch?
Regards,
Dmitry

[DB] Calculations in the ipmi_intf_get_max_request_data_size() are
required for the case if the target IPMC device is accessed via IPMI
bridging. Since we can not deduce the target channel maximum message
size, we use the minimum required size. These calculations are not
needed for direct IPMC device access.
[DB] Set max size functions are required if maximum message size over
the chosen interface must be somehow modified from the value received
from the interface properties. This is the case for the encrypted RMCP+
payload where maximum message size must be reduced by the
confidentiality header/trailer sizes. Other interface types do not even
implement these callbacks.

Post by Zdenek Styblik
~~~
+ if (rsp) {
+ if (rsp->ccode == 0xC1) {
+ lprintf(LOG_DEBUG,"IPM Controller is not HPM.2 compatible");
+ return rsp->ccode;
+ } else if (rsp->ccode) {
+ lprintf(LOG_NOTICE,"Get HPM.x Capabilities request failed,"
+ " compcode = %x\n", rsp->ccode);
+ return rsp->ccode;
+ }
+ } else {
+ lprintf(LOG_NOTICE,"Error sending request\n");
+ return -1;
+ }
~~~
This seems to have been missed. :)

[DB] Fixed.

Post by Zdenek Styblik
~~~
if (my_long_packet_set == 1) {
+ if (ipmi_oem_active(ipmi_main_intf, "kontron"))
+ {
/* Restore defaults */
ipmi_kontronoem_set_large_buffer( ipmi_main_intf, 0 );
+ }
~~~
This seems to be have strange indentation.

[DB] Fixed.

Post by Zdenek Styblik
Other than these and code formatting, I guess it's ok.
Have a nice weekend,
Z.

[DB] My late thanks).
[DB] If there will be no additional review comments, could you
explicitly confirm acceptance of the patch? If such, I will re-base my
repository in order to prepare subsequent feature patches.
[DB] No more comments below. Regards, Dmitry

Post by Dmitry Bazhenov
Hello, Zdenek,
I addressed your comments. Please, review once more.
Regards,
Dmitry

Post by Zdenek Styblik
On Tue, Feb 18, 2014 at 11:07 AM, Dmitry Bazhenov
[...]

Currently, only utility functions. In the future there will be some
commands. I'll rename the files.

Good.

[...]

Post by Dmitry Bazhenov
"Extern" is the default storage class for functions in C. In this example
the extern linkage is just stated explicitly.

Ok.

Post by Zdenek Styblik
* lib/ipmi_hpm2.c
~~~
+ if (rsp) {
+ if (rsp->ccode == 0xC1) {
+ lprintf(LOG_DEBUG,"IPM Controller is not HPM.2 compatible");
+ return rsp->ccode;
+ } else if (rsp->ccode) {
+ lprintf(LOG_NOTICE,"Get HPM.x Capabilities request failed,"
+ " compcode = %x\n", rsp->ccode);
+ return rsp->ccode;
+ }
+ } else {
+ lprintf(LOG_NOTICE,"Error sending request\n");
+ return -1;
+ }
~~~
->
~~~
if (rsp == NULL) {
lprintf(LOG_NOTICE,"Error sending request\n");
return -1;
}
+ if (rsp->ccode == 0xC1) {
+ lprintf(LOG_DEBUG,"IPM Controller is not HPM.2 compatible");
+ return rsp->ccode;
+ } else if (rsp->ccode) {
+ lprintf(LOG_NOTICE,"Get HPM.x Capabilities request failed,"
+ " compcode = %x\n", rsp->ccode);
+ return rsp->ccode;
+ }
[ continue with the code in function
~~~
~~~
if (caps->lan_channel_mask) {
[...]
}
return 0;
~~~
->
~~~
if (!caps->lan_channel_mask) {
return 0;
}
[ continue with code from if() block here ]
~~~
This repeats couple times through the file in question.
* lib/ipmi_hpmfwupg.c
~~~
+ uploadCmd.req =
malloc(ipmi_intf_get_max_request_data_size(intf));
~~~
This doesn't look like a good idea. You just shouldn't do things like this.

I do not understand the argument. Please, elaborate more on this. The code
does what is required. What's wrong?

The argument is - it's just over-complicated. Please, return as early
possible and don't do if() for if().
~~~
if (rsp == NULL) {
/* error */
return (-1);
}
if (rsp->ccode > 0) {
/* different error */
return rsp->ccode;
}
~~~

Post by Zdenek Styblik
~~~
+ uploadCmd.req =
malloc(ipmi_intf_get_max_request_data_size(intf));
~~~

~~~
int allocate_i = ipmi_intf_get_max_request_data_size(intf);
uploadCmd.req = malloc(allocate_i);
~~~
I mean, don't use return value directly as an argument for function.

No at all. Boards may support HPM.2 and may not. If they are, the function
will set appropriate payload sizes.

Ok.

This functions are for the internal module use. Using static storage type is
logical here.

Yeah, we had several discussions with friend of mine. It turned out we
should reconsider our view on 'static'. So, agreed. My bad.

Post by Zdenek Styblik
* src/plugins/open/open.c
~~~
+#define IPMI_OPENIPMI_DEFAULT_PAYLOAD_SIZE 37
~~~
I hope this doesn't break anything :)

This surely does not break for requests, but can break for responses. I'll
fix it.

Haha, not what I meant, but great :)

I'll add necessary checks where it is appropriate.

Good.
And I'm sorry for delay. It took some time to discuss 'static' thing,
but the fact is, I just kept this reply on back-burner. If this ever
happens again, just kick me to see whether I'm alive.
Thanks,
Z.

Zdenek Styblik

2014-03-20 05:54:26 UTC

On Wed, Mar 19, 2014 at 8:33 AM, Dmitry Bazhenov <***@pigeonpoint.com> wrote:
[...]

Post by Zdenek Styblik
I got a bit "scared" by solution applied to
ipmi_intf_get_max_request_data_size() and
ipmi_intf_get_max_response_data_size(). But then I've tried to compile
just this one function with all kinds of switches and compiler didn't
comply, so I guess it's ok.
I wonder, shouldn't be the same logic applied to
ipmi_lanp_set_max_rq_data_size() and ipmi_lanp_set_max_rp_data_size()
as well?

[DB] Calculations in the ipmi_intf_get_max_request_data_size() are required
for the case if the target IPMC device is accessed via IPMI bridging. Since
we can not deduce the target channel maximum message size, we use the
minimum required size. These calculations are not needed for direct IPMC
device access.
[DB] Set max size functions are required if maximum message size over the
chosen interface must be somehow modified from the value received from the
interface properties. This is the case for the encrypted RMCP+ payload where
maximum message size must be reduced by the confidentiality header/trailer
sizes. Other interface types do not even implement these callbacks.

What I meant is whether under/over-flow shouldn't be checked in those
functions as well.

[...]

[DB] My late thanks).
[DB] If there will be no additional review comments, could you explicitly
confirm acceptance of the patch? If such, I will re-base my repository in
order to prepare subsequent feature patches.

I pretty much guess there won't be any more comments on my side.

Take care,
Z.

[DB] No more comments below. Regards, Dmitry

Post by Dmitry Bazhenov
Hello, Zdenek,
I addressed your comments. Please, review once more.
Regards,
Dmitry

Post by Zdenek Styblik
On Tue, Feb 18, 2014 at 11:07 AM, Dmitry Bazhenov
[...]

Currently, only utility functions. In the future there will be some
commands. I'll rename the files.

Good.

[...]

Post by Dmitry Bazhenov
"Extern" is the default storage class for functions in C. In this example
the extern linkage is just stated explicitly.

Ok.

Post by Zdenek Styblik
* lib/ipmi_hpm2.c
~~~
+ if (rsp) {
+ if (rsp->ccode == 0xC1) {
+ lprintf(LOG_DEBUG,"IPM Controller is not HPM.2 compatible");
+ return rsp->ccode;
+ } else if (rsp->ccode) {
+ lprintf(LOG_NOTICE,"Get HPM.x Capabilities request failed,"
+ " compcode = %x\n", rsp->ccode);
+ return rsp->ccode;
+ }
+ } else {
+ lprintf(LOG_NOTICE,"Error sending request\n");
+ return -1;
+ }
~~~
->
~~~
if (rsp == NULL) {
lprintf(LOG_NOTICE,"Error sending request\n");
return -1;
}
+ if (rsp->ccode == 0xC1) {
+ lprintf(LOG_DEBUG,"IPM Controller is not HPM.2 compatible");
+ return rsp->ccode;
+ } else if (rsp->ccode) {
+ lprintf(LOG_NOTICE,"Get HPM.x Capabilities request failed,"
+ " compcode = %x\n", rsp->ccode);
+ return rsp->ccode;
+ }
[ continue with the code in function
~~~
~~~
if (caps->lan_channel_mask) {
[...]
}
return 0;
~~~
->
~~~
if (!caps->lan_channel_mask) {
return 0;
}
[ continue with code from if() block here ]
~~~
This repeats couple times through the file in question.
* lib/ipmi_hpmfwupg.c
~~~
+ uploadCmd.req =
malloc(ipmi_intf_get_max_request_data_size(intf));
~~~
This doesn't look like a good idea. You just shouldn't do things like this.

I do not understand the argument. Please, elaborate more on this. The code
does what is required. What's wrong?

The argument is - it's just over-complicated. Please, return as early
possible and don't do if() for if().
~~~
if (rsp == NULL) {
/* error */
return (-1);
}
if (rsp->ccode > 0) {
/* different error */
return rsp->ccode;
}
~~~

Post by Zdenek Styblik
~~~
+ uploadCmd.req =
malloc(ipmi_intf_get_max_request_data_size(intf));
~~~

~~~
int allocate_i = ipmi_intf_get_max_request_data_size(intf);
uploadCmd.req = malloc(allocate_i);
~~~
I mean, don't use return value directly as an argument for function.

No at all. Boards may support HPM.2 and may not. If they are, the function
will set appropriate payload sizes.

Ok.

This functions are for the internal module use. Using static storage type is
logical here.

Yeah, we had several discussions with friend of mine. It turned out we
should reconsider our view on 'static'. So, agreed. My bad.

Post by Zdenek Styblik
* src/plugins/open/open.c
~~~
+#define IPMI_OPENIPMI_DEFAULT_PAYLOAD_SIZE 37
~~~
I hope this doesn't break anything :)

This surely does not break for requests, but can break for responses. I'll
fix it.

Haha, not what I meant, but great :)

I'll add necessary checks where it is appropriate.

Good.
And I'm sorry for delay. It took some time to discuss 'static' thing,
but the fact is, I just kept this reply on back-burner. If this ever
happens again, just kick me to see whether I'm alive.
Thanks,
Z.

Zdenek Styblik

2014-03-31 07:07:23 UTC

On Thu, Mar 20, 2014 at 6:54 AM, Zdenek Styblik

[DB] Calculations in the ipmi_intf_get_max_request_data_size() are required
for the case if the target IPMC device is accessed via IPMI bridging. Since
we can not deduce the target channel maximum message size, we use the
minimum required size. These calculations are not needed for direct IPMC
device access.
[DB] Set max size functions are required if maximum message size over the
chosen interface must be somehow modified from the value received from the
interface properties. This is the case for the encrypted RMCP+ payload where
maximum message size must be reduced by the confidentiality header/trailer
sizes. Other interface types do not even implement these callbacks.

What I meant is whether under/over-flow shouldn't be checked in those
functions as well.

Ping?

Z.

Dmitry Bazhenov

2014-03-31 07:14:56 UTC

Hello, Zdenek,

I think there should be no such checks inside these callbacks.
However, I guess there should be a check inside thr
ipmi_intf_set_max_request/response_data_size
functions which guarantee that the minimum value will be not less than
25 bytes (required by IPMI spec).

Could you please add such check or is it better for me to provide a new
patch revision?

Regards,
Dmitry

Post by Zdenek Styblik
On Thu, Mar 20, 2014 at 6:54 AM, Zdenek Styblik

[DB] Calculations in the ipmi_intf_get_max_request_data_size() are required
for the case if the target IPMC device is accessed via IPMI bridging. Since
we can not deduce the target channel maximum message size, we use the
minimum required size. These calculations are not needed for direct IPMC
device access.
[DB] Set max size functions are required if maximum message size over the
chosen interface must be somehow modified from the value received from the
interface properties. This is the case for the encrypted RMCP+ payload where
maximum message size must be reduced by the confidentiality header/trailer
sizes. Other interface types do not even implement these callbacks.

What I meant is whether under/over-flow shouldn't be checked in those
functions as well.

Ping?
Z.

Zdenek Styblik

2014-03-31 07:34:45 UTC

Post by Dmitry Bazhenov
Hello, Zdenek,
I think there should be no such checks inside these callbacks.
However, I guess there should be a check inside thr
ipmi_intf_set_max_request/response_data_size
functions which guarantee that the minimum value will be not less than 25
bytes (required by IPMI spec).
Could you please add such check or is it better for me to provide a new
patch revision?
Regards,
Dmitry

Dmitry,

I don't have access to any IPMI capable hardware, so I'm afraid it's
either up to you or somebody else. I'm sorry.

Best regards,
Z.

Post by Zdenek Styblik
On Thu, Mar 20, 2014 at 6:54 AM, Zdenek Styblik

[DB] Calculations in the ipmi_intf_get_max_request_data_size() are required
for the case if the target IPMC device is accessed via IPMI bridging. Since
we can not deduce the target channel maximum message size, we use the
minimum required size. These calculations are not needed for direct IPMC
device access.
[DB] Set max size functions are required if maximum message size over the
chosen interface must be somehow modified from the value received from the
interface properties. This is the case for the encrypted RMCP+ payload where
maximum message size must be reduced by the confidentiality
header/trailer
sizes. Other interface types do not even implement these callbacks.

What I meant is whether under/over-flow shouldn't be checked in those
functions as well.

Ping?
Z.

Dmitry Bazhenov

2014-03-31 07:37:56 UTC

Zdenek,

Okay then. I'll provide the updated patch later today.

Regards,
Dmitry

Dmitry,
I don't have access to any IPMI capable hardware, so I'm afraid it's
either up to you or somebody else. I'm sorry.
Best regards,
Z.

Post by Zdenek Styblik
On Thu, Mar 20, 2014 at 6:54 AM, Zdenek Styblik

[DB] Calculations in the ipmi_intf_get_max_request_data_size() are required
for the case if the target IPMC device is accessed via IPMI bridging. Since
we can not deduce the target channel maximum message size, we use the
minimum required size. These calculations are not needed for direct IPMC
device access.
[DB] Set max size functions are required if maximum message size over the
chosen interface must be somehow modified from the value received from the
interface properties. This is the case for the encrypted RMCP+ payload where
maximum message size must be reduced by the confidentiality header/trailer
sizes. Other interface types do not even implement these callbacks.

What I meant is whether under/over-flow shouldn't be checked in those
functions as well.

Ping?
Z.

Dmitry Bazhenov

2014-03-31 16:28:20 UTC

Zdenek,

Here is the updated patch.

Regards,
Dmitry

Post by Dmitry Bazhenov
Zdenek,
Okay then. I'll provide the updated patch later today.
Regards,
Dmitry

On Mon, Mar 31, 2014 at 9:14 AM, Dmitry Bazhenov

Dmitry,
I don't have access to any IPMI capable hardware, so I'm afraid it's
either up to you or somebody else. I'm sorry.
Best regards,
Z.

Post by Zdenek Styblik
On Thu, Mar 20, 2014 at 6:54 AM, Zdenek Styblik

Post by Zdenek Styblik
On Wed, Mar 19, 2014 at 8:33 AM, Dmitry Bazhenov
[...]

[DB] Calculations in the ipmi_intf_get_max_request_data_size() are required
for the case if the target IPMC device is accessed via IPMI
bridging.
Since
we can not deduce the target channel maximum message size, we use the
minimum required size. These calculations are not needed for direct IPMC
device access.
[DB] Set max size functions are required if maximum message size over
the
chosen interface must be somehow modified from the value received
from
the
interface properties. This is the case for the encrypted RMCP+
payload
where
maximum message size must be reduced by the confidentiality header/trailer
sizes. Other interface types do not even implement these callbacks.

What I meant is whether under/over-flow shouldn't be checked in those
functions as well.

Ping?
Z.

Dmitry Bazhenov

2014-04-17 11:55:18 UTC

Dmitry Bazhenov

2014-05-16 12:55:22 UTC

<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">Hello, all, 
 
Can I expect any progress on the posted patches? 
<pre class="moz-signature" cols="72">With regards,
Dmitry</pre>
17.04.2014 17:55, Dmitry Bazhenov пишет: 
</div>
<blockquote cite="mid:***@pigeonpoint.com" type="cite">
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
<div class="moz-cite-prefix">Hello, ipmitool maintainers, 
 
I would like to submit several patches which adds some new
functionality into ipmitool, as well as fix some bugs. 
 
1. <a moz-do-not-send="true" class="alink"
href="http://sourceforge.net/p/ipmitool/bugs/305/">[bugs:#305]</a>
deferred-activation-fix.diff 
 This patch fixes the ipmitool HPM.1 agent which
mis-recognizes the deferred activation support and reports
invalid deferred firmware image version. 
 
2. <a moz-do-not-send="true" class="alink"
href="http://sourceforge.net/p/ipmitool/bugs/306/">[bugs:#306]</a>
fru-info-fix.diff 
 This patch removes duplicate output of FRU info #0 when
command fru print all is sent. 
3. <a moz-do-not-send="true" class="alink"
href="http://sourceforge.net/p/ipmitool/bugs/307/">[bugs:#307]</a>
i82751spt-fix.diff 
 This patch adds missing check in the LAN+ implementation for
Intel i82751 MAC which has known deviations from the IPMI v2.0
specification. 
 
4. <a moz-do-not-send="true" class="alink"
href="http://sourceforge.net/p/ipmitool/patches/94/">[patches:#94]</a>
vita-support.diff 
 This patch adds VITA 46.11 specification support to
ipmitool. 
 
5. <a moz-do-not-send="true" class="alink"
href="http://sourceforge.net/p/ipmitool/patches/95/">[patches:#95]</a>
intf-reopen-fix.diff 
 This patch provides a solution how to overcome the
architectural ipmitool drawback which 
 makes impossible to normally (without hacks) close and
re-open interface. 
 
Please, review. 

<pre class="moz-signature" cols="72">Regards,
Dmitry</pre>
31.03.2014 22:28, Dmitry Bazhenov пишет: 
</div>
<blockquote cite="mid:***@pigeonpoint.com"
type="cite">Zdenek, 
 
Here is the updated patch. 
 
Regards, 
Dmitry 
 
31.03.2014 13:37, Dmitry Bazhenov пишет: 
<blockquote type="cite">Zdenek, 
 
Okay then. I'll provide the updated patch later today. 
 
Regards, 
Dmitry 
 
31.03.2014 13:34, Zdenek Styblik пишет: 
<blockquote type="cite">On Mon, Mar 31, 2014 at 9:14 AM,
Dmitry Bazhenov <a moz-do-not-send="true"
class="moz-txt-link-rfc2396E"
href="mailto:***@pigeonpoint.com"><***@pigeonpoint.com></a>
wrote: 
<blockquote type="cite">Hello, Zdenek, 
 
I think there should be no such checks inside these
callbacks. 
However, I guess there should be a check inside thr 
ipmi_intf_set_max_request/response_data_size 
functions which guarantee that the minimum value will be
not less than 25 
bytes (required by IPMI spec). 
 
Could you please add such check or is it better for me to
provide a new 
patch revision? 
 
Regards, 
Dmitry 
 
</blockquote>
Dmitry, 
 
I don't have access to any IPMI capable hardware, so I'm
afraid it's 
either up to you or somebody else. I'm sorry. 
 
Best regards, 
Z. 
 
<blockquote type="cite">31.03.2014 13:07, Zdenek Styblik
пишет: 
 
<blockquote type="cite">On Thu, Mar 20, 2014 at 6:54 AM,
Zdenek Styblik 
<a moz-do-not-send="true" class="moz-txt-link-rfc2396E"
href="mailto:***@gmail.com"><***@gmail.com></a>
wrote: 
<blockquote type="cite">On Wed, Mar 19, 2014 at 8:33 AM,
Dmitry Bazhenov <a moz-do-not-send="true"
class="moz-txt-link-rfc2396E"
href="mailto:***@pigeonpoint.com"><***@pigeonpoint.com></a>
 
wrote: 
[...] 
<blockquote type="cite">
<blockquote type="cite">I got a bit "scared" by
solution applied to 
ipmi_intf_get_max_request_data_size() and 
ipmi_intf_get_max_response_data_size(). But then
I've tried to compile 
just this one function with all kinds of switches
and compiler didn't 
comply, so I guess it's ok. 
I wonder, shouldn't be the same logic applied to 
ipmi_lanp_set_max_rq_data_size() and
ipmi_lanp_set_max_rp_data_size() 
as well? 
</blockquote>
[DB] Calculations in the
ipmi_intf_get_max_request_data_size() are 
required 
for the case if the target IPMC device is accessed
via IPMI bridging. 
Since 
we can not deduce the target channel maximum message
size, we use the 
minimum required size. These calculations are not
needed for direct IPMC 
device access. 
[DB] Set max size functions are required if maximum
message size over 
the 
chosen interface must be somehow modified from the
value received from 
the 
interface properties. This is the case for the
encrypted RMCP+ payload 
where 
maximum message size must be reduced by the
confidentiality 
header/trailer 
sizes. Other interface types do not even implement
these callbacks. 
 
</blockquote>
What I meant is whether under/over-flow shouldn't be
checked in those 
functions as well. 
 
</blockquote>
Ping? 
 
Z. 
</blockquote>
 
</blockquote>
</blockquote>
 
</blockquote>
 
</blockquote>
 
</blockquote>
 
</body>
</html>

Zdenek Styblik

2014-05-16 13:33:06 UTC

Post by Dmitry Bazhenov
Hello, all,
Can I expect any progress on the posted patches?
With regards,
Dmitry

Sure,

send over some beers ;)

Z.

--
Zdenek Styblik

Post by Dmitry Bazhenov
Hello, ipmitool maintainers,
I would like to submit several patches which adds some new functionality
into ipmitool, as well as fix some bugs.
1. [bugs:#305] deferred-activation-fix.diff
This patch fixes the ipmitool HPM.1 agent which mis-recognizes the
deferred activation support and reports invalid deferred firmware image
version.
2. [bugs:#306] fru-info-fix.diff
This patch removes duplicate output of FRU info #0 when command fru
print all is sent.
3. [bugs:#307] i82751spt-fix.diff
This patch adds missing check in the LAN+ implementation for Intel
i82751 MAC which has known deviations from the IPMI v2.0 specification.
4. [patches:#94] vita-support.diff
This patch adds VITA 46.11 specification support to ipmitool.
5. [patches:#95] intf-reopen-fix.diff
This patch provides a solution how to overcome the architectural ipmitool
drawback which
makes impossible to normally (without hacks) close and re-open interface.
Please, review.
Regards,
Dmitry
Zdenek,
Here is the updated patch.
Regards,
Dmitry
Zdenek,
Okay then. I'll provide the updated patch later today.
Regards,
Dmitry
Hello, Zdenek,
I think there should be no such checks inside these callbacks.
However, I guess there should be a check inside thr
ipmi_intf_set_max_request/response_data_size
functions which guarantee that the minimum value will be not less than 25
bytes (required by IPMI spec).
Could you please add such check or is it better for me to provide a new
patch revision?
Regards,
Dmitry
Dmitry,
I don't have access to any IPMI capable hardware, so I'm afraid it's
either up to you or somebody else. I'm sorry.
Best regards,
Z.
On Thu, Mar 20, 2014 at 6:54 AM, Zdenek Styblik
[...]
I got a bit "scared" by solution applied to
ipmi_intf_get_max_request_data_size() and
ipmi_intf_get_max_response_data_size(). But then I've tried to compile
just this one function with all kinds of switches and compiler didn't
comply, so I guess it's ok.
I wonder, shouldn't be the same logic applied to
ipmi_lanp_set_max_rq_data_size() and ipmi_lanp_set_max_rp_data_size()
as well?
[DB] Calculations in the ipmi_intf_get_max_request_data_size() are required
for the case if the target IPMC device is accessed via IPMI bridging. Since
we can not deduce the target channel maximum message size, we use the
minimum required size. These calculations are not needed for direct IPMC
device access.
[DB] Set max size functions are required if maximum message size over the
chosen interface must be somehow modified from the value received from the
interface properties. This is the case for the encrypted RMCP+ payload where
maximum message size must be reduced by the confidentiality
header/trailer
sizes. Other interface types do not even implement these callbacks.
What I meant is whether under/over-flow shouldn't be checked in those
functions as well.
Ping?
Z.
------------------------------------------------------------------------------
"Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
Instantly run your Selenium tests across 300+ browser/OS combos.
Get unparalleled scalability from the best Selenium testing platform
available
Simple to use. Nothing to install. Get started now for free."
http://p.sf.net/sfu/SauceLabs
_______________________________________________
Ipmitool-devel mailing list
https://lists.sourceforge.net/lists/listinfo/ipmitool-devel

Dmitry Bazhenov

2014-07-25 05:39:23 UTC

Hello, Zdenek,

Could you or someone else from the development team comment
on the acceptance of the following bugfix/feature patches:

#307 Intel I82751 super pass through mode fixup
#319 Interface safe re-open.
#320 VITA 46.11 support
#328 HPM.2 fixes
#329 hpm.1 upgrade fixes

There have been no comments on them for two months already.
We would want them incorporated into the mainline sources before the
1.8.15 release if possible.

Will appreciate any feedback.

Regards,
Dmitry

Post by Dmitry Bazhenov
Hello, all,
Can I expect any progress on the posted patches?
With regards,
Dmitry

Sure,
send over some beers ;)
Z.
--
Zdenek Styblik

Zdenek Styblik

2014-07-28 09:00:52 UTC

Post by Dmitry Bazhenov
Hello, Zdenek,

Hello Dmitry,

Post by Dmitry Bazhenov
Could you or someone else from the development team comment

if I had time to do so, I'd have done it already. And I have no ETA
when I'll have time to get back to IPMI tool again, at least not now.

[...]

Post by Dmitry Bazhenov
There have been no comments on them for two months already.

There have been no activity what so ever for two months already. It
would be worth to ask why and perhaps start looking for some solution,
wouldn't it? It's not just patches from Pigeonpoint, but bug reports
and patches from other users as well.
I simply don't have any free time left to do this stuff at the moment.
And forgive me for putting a "hobby" on the back-burner. Perhaps even
further than that.
I've also noticed the past couple months a demand for almost an
enterprise level of response times on code review and integration has
appeared. I fully understand reasoning behind it and I believe it's
even good for project itself. I mean, that's great, because it means
somebody is using it and cares about it(there were couple bug reports
from other users as well - great!). However, let me put one thing
straight. It's not going to happen on my side, for free, in my free
time - *EVER*. I'm not 20 years old student anymore. Sorry.
I'm open to talk about kickstarter, sponsoring, even being
hired(although under very, very specific conditions); in order to cut
on other activities(= job[s]) and shift focus to IPMI tool. This
project would use at least one full time and vendor independent
programmer.
Now, I have a better idea. How about you guys drop this
I-can't-talk-to-other-guys-because-they're-competitors malarkey and
start cooperating? And I truly mean cooperating. Also,
improving/cleaning up/fixing the code itself, not just integrating
more and more bloat on top of it. How about to convince your managers
and their managers and so on about this? Linux kernel is being/getting
done this way, doesn't it? Also, Linus and Greg K.H. are being paid by
Linux Foundation, so neutrality, or independent mediators, is secured.
Hopefully, you know what I mean.
Because I simply can't see other way for this project. Either way,
somebody has to pay (for dev time). Nothing comes for free.
Kontron used to have paid developers dedicated to this project, but I
guess their focus have shifted.

Post by Dmitry Bazhenov
Will appreciate any feedback.

One feedback, though. Fix code formatting, if nothing else. I'm not
going to do it for you, or anybody else, anymore. It got tiresome
after 1+ year of doing so; I can do better things than that; and I
don't have that much of free time to do so(see previous reason).

Sorry for bad news, but reality check was necessary from my point of view.

Best regards,
Z.

Post by Dmitry Bazhenov
Regards,
Dmitry

Post by Dmitry Bazhenov
Hello, all,
Can I expect any progress on the posted patches?
With regards,
Dmitry

Sure,
send over some beers ;)
Z.
--
Zdenek Styblik

Dmitry Bazhenov

2014-07-28 14:15:15 UTC

Hello, Zdenek,

Please, see below.

Post by Dmitry Bazhenov
Hello, Zdenek,

Hello Dmitry,

Post by Dmitry Bazhenov
Could you or someone else from the development team comment

if I had time to do so, I'd have done it already. And I have no ETA
when I'll have time to get back to IPMI tool again, at least not now.

Understood.

Post by Dmitry Bazhenov
There have been no comments on them for two months already.

It's all understood. We did our ipmitool bug-fixes on a paid basis too,
so I understand
your position. I'll pass the response to my employers in order to get
the plan how to
move patches further.

Post by Dmitry Bazhenov
Will appreciate any feedback.

One feedback, though. Fix code formatting, if nothing else. I'm not
going to do it for you, or anybody else, anymore. It got tiresome
after 1+ year of doing so; I can do better things than that; and I
don't have that much of free time to do so(see previous reason).
Sorry for bad news, but reality check was necessary from my point of view.

Nevermind. Thanks for the support anyway, we appreciate it.

On more thing, can you tell if anyone else beside you administrates
(actively) the project?

Regards,
Dmitry

Post by Zdenek Styblik
Best regards,
Z.

Post by Dmitry Bazhenov
Regards,
Dmitry

Post by Dmitry Bazhenov
Hello, all,
Can I expect any progress on the posted patches?
With regards,
Dmitry

Sure,
send over some beers ;)
Z.
--
Zdenek Styblik

Zdenek Styblik

2014-07-28 18:11:42 UTC

On Mon, Jul 28, 2014 at 4:15 PM, Dmitry Bazhenov <***@pigeonpoint.com> wrote:
[...]

It's all understood. We did our ipmitool bug-fixes on a paid basis too, so I
understand your position.

Everybody else has the same deal and it makes sense from certain
business point of view. It depends what your product is, what drives
your customers and so on. But you can say somebody from corporation A
pops up, spends some time with something, and gets pulled to other
stuff.

I'll pass the response to my employers in order to get the
plan how to
move patches further.

Please, don't get me wrong. I don't blame Pigeonpoint or you. I meant
it in general and pitching/asking for long term solution. Pretty much
all vendors should give this a thought as they're all affected one way
or another. One guy doing this in free time doesn't seem like a good
solution.
If I ever manage to un-buffer enough, I'll try to get back. It might
be next weekend, next moth or never. For better or worse, some changes
are coming in a month or so.
However, in my opinion, this project needs more than just integration
of "new features". But it's just my opinion. However, should it
continue by just integration and snowballing, then I'm no longer
needed here anymore and can move on.

[...]

On more thing, can you tell if anyone else beside you administrates
(actively) the project?

Administration like to grant commit privileges? Yes. Participating on
the project? No.
Majority of those people are/used to be from Kontron and my guess is
Kontron did whatever it needed to with IPMI tool, say an integration
of sorts, and moved on.

Have a nice evening,
Z.

Regards,
Dmitry

Post by Zdenek Styblik
Best regards,
Z.

Post by Dmitry Bazhenov
Regards,
Dmitry

On Fri, May 16, 2014 at 2:55 PM, Dmitry Bazhenov

Post by Dmitry Bazhenov
Hello, all,
Can I expect any progress on the posted patches?
With regards,
Dmitry

Sure,
send over some beers ;)
Z.
--
Zdenek Styblik

Post by Dmitry Bazhenov
Hello, ipmitool maintainers,
I would like to submit several patches which adds some new
functionality
into ipmitool, as well as fix some bugs.
1. [bugs:#305] deferred-activation-fix.diff
This patch fixes the ipmitool HPM.1 agent which mis-recognizes the
deferred activation support and reports invalid deferred firmware image
version.
2. [bugs:#306] fru-info-fix.diff
This patch removes duplicate output of FRU info #0 when command fru
print all is sent.
3. [bugs:#307] i82751spt-fix.diff
This patch adds missing check in the LAN+ implementation for Intel
i82751 MAC which has known deviations from the IPMI v2.0 specification.
4. [patches:#94] vita-support.diff
This patch adds VITA 46.11 specification support to ipmitool.
5. [patches:#95] intf-reopen-fix.diff
This patch provides a solution how to overcome the architectural ipmitool
drawback which
makes impossible to normally (without hacks) close and re-open interface.
Please, review.
Regards,
Dmitry
Zdenek,
Here is the updated patch.
Regards,
Dmitry
Zdenek,
Okay then. I'll provide the updated patch later today.
Regards,
Dmitry
On Mon, Mar 31, 2014 at 9:14 AM, Dmitry Bazhenov
Hello, Zdenek,
I think there should be no such checks inside these callbacks.
However, I guess there should be a check inside thr
ipmi_intf_set_max_request/response_data_size
functions which guarantee that the minimum value will be not less than 25
bytes (required by IPMI spec).
Could you please add such check or is it better for me to provide a new
patch revision?
Regards,
Dmitry
Dmitry,
I don't have access to any IPMI capable hardware, so I'm afraid it's
either up to you or somebody else. I'm sorry.
Best regards,
Z.
On Thu, Mar 20, 2014 at 6:54 AM, Zdenek Styblik
On Wed, Mar 19, 2014 at 8:33 AM, Dmitry Bazhenov
[...]
I got a bit "scared" by solution applied to
ipmi_intf_get_max_request_data_size() and
ipmi_intf_get_max_response_data_size(). But then I've tried to compile
just this one function with all kinds of switches and compiler didn't
comply, so I guess it's ok.
I wonder, shouldn't be the same logic applied to
ipmi_lanp_set_max_rq_data_size() and ipmi_lanp_set_max_rp_data_size()
as well?
[DB] Calculations in the ipmi_intf_get_max_request_data_size() are required
for the case if the target IPMC device is accessed via IPMI bridging. Since
we can not deduce the target channel maximum message size, we use the
minimum required size. These calculations are not needed for direct IPMC
device access.
[DB] Set max size functions are required if maximum message size over the
chosen interface must be somehow modified from the value received from the
interface properties. This is the case for the encrypted RMCP+ payload where
maximum message size must be reduced by the confidentiality header/trailer
sizes. Other interface types do not even implement these callbacks.
What I meant is whether under/over-flow shouldn't be checked in those
functions as well.
Ping?
Z.
------------------------------------------------------------------------------
"Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
Instantly run your Selenium tests across 300+ browser/OS combos.
Get unparalleled scalability from the best Selenium testing platform
available
Simple to use. Nothing to install. Get started now for free."
http://p.sf.net/sfu/SauceLabs
_______________________________________________
Ipmitool-devel mailing list
https://lists.sourceforge.net/lists/listinfo/ipmitool-devel

Dmitry Bazhenov

2014-08-05 12:44:59 UTC

<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
Hank,
Zdenek and others:
<div dir="ltr" align="left">
<div dir="ltr" align="left"> </div>
<div dir="ltr" align="left">Here is a message on this
topic that PPS management asked me to forward to this
reflector. 
--- 
</div>
<div dir="ltr" align="left"></div>
<div dir="ltr" align="left">As we've said previously, it is
certainly understandable if Zdenek no longer has the
bandwidth to invest in this area; we continue to thank you
for your efforts thus far, Zdenek!</div>
<div dir="ltr" align="left"> </div>
<div dir="ltr" align="left">We have made a substantial
investment in ipmitool over the years, with development
of the patches that triggered this discussion being just a
small part, but we are not prepared to take over
maintenance of ipmitool for the entire community. We'll
continue to assist our customers in the relevant contexts
for use of this valuable tool, but we don't have the
bandwidth to have a broader involvement than that.</div>
<div dir="ltr" align="left"> </div>
<div dir="ltr" align="left">If one or more folks or
companies step forward to take this responsibility, we'll
be pleased to cooperate with them, as we have done to this
point. Pending that, we'll focus on supporting
our own customers.</div>
<div dir="ltr" align="left"> </div>
<div dir="ltr" align="left"><o:p>Thanks again, Zdenek for your
contributions thus far.</o:p></div>
<div dir="ltr" align="left"><o:p></o:p> </div>
<div dir="ltr" align="left"><o:p>Mark Overgaard 
</o:p>
<div dir="ltr" align="left">--- 
</div>
<div dir="ltr" align="left">Regards, 
Dmitry 
</div>
 
</div>
</div>
29.07.2014 0:11, Zdenek Styblik пишет: 
<blockquote
cite="mid:***@mail.gmail.com"
type="cite"> <pre wrap="">On Mon, Jul 28, 2014 at 4:15 PM, Dmitry Bazhenov <a class="moz-txt-link-rfc2396E" href="mailto:***@pigeonpoint.com"><***@pigeonpoint.com></a> wrote:
[...]
</pre>
<blockquote type="cite">
<pre wrap="">
It's all understood. We did our ipmitool bug-fixes on a paid basis too, so I
understand your position.
</pre>
</blockquote>
<pre wrap="">
Everybody else has the same deal and it makes sense from certain
business point of view. It depends what your product is, what drives
your customers and so on. But you can say somebody from corporation A
pops up, spends some time with something, and gets pulled to other
stuff.

</pre>
<blockquote type="cite">
<pre wrap="">I'll pass the response to my employers in order to get the
plan how to
move patches further.

</pre>
</blockquote>
<pre wrap="">
Please, don't get me wrong. I don't blame Pigeonpoint or you. I meant
it in general and pitching/asking for long term solution. Pretty much
all vendors should give this a thought as they're all affected one way
or another. One guy doing this in free time doesn't seem like a good
solution.
If I ever manage to un-buffer enough, I'll try to get back. It might
be next weekend, next moth or never. For better or worse, some changes
are coming in a month or so.
However, in my opinion, this project needs more than just integration
of "new features". But it's just my opinion. However, should it
continue by just integration and snowballing, then I'm no longer
needed here anymore and can move on.

[...]
</pre>
<blockquote type="cite">
<pre wrap="">
On more thing, can you tell if anyone else beside you administrates
(actively) the project?
</pre>
</blockquote>
<pre wrap="">
Administration like to grant commit privileges? Yes. Participating on
the project? No.
Majority of those people are/used to be from Kontron and my guess is
Kontron did whatever it needed to with IPMI tool, say an integration
of sorts, and moved on.

Have a nice evening,
Z.

</pre>
<blockquote type="cite">
<pre wrap="">
Regards,
Dmitry

</pre>
<blockquote type="cite">
<pre wrap="">
Best regards,
Z.

</pre>
<blockquote type="cite">
<pre wrap="">Regards,
Dmitry

16.05.2014 19:33, Zdenek Styblik ?????: </pre> <blockquote type="cite"> <pre wrap="">On Fri, May 16, 2014 at 2:55 PM, Dmitry Bazhenov <a class="moz-txt-link-rfc2396E" href="mailto:***@pigeonpoint.com"><***@pigeonpoint.com></a>
wrote:
</pre>
<blockquote type="cite">
<pre wrap="">
Hello, all,

Can I expect any progress on the posted patches?

With regards,
Dmitry

</pre>
</blockquote>
<pre wrap="">Sure,

send over some beers ;)

Z.

--
Zdenek Styblik
email: <a class="moz-txt-link-abbreviated" href="mailto:***@gmail.com">***@gmail.com</a>
jabber: <a class="moz-txt-link-abbreviated" href="mailto:***@gmail.com">***@gmail.com</a>

</pre>
<blockquote type="cite">
<pre wrap="">17.04.2014 17:55, Dmitry Bazhenov пишет:

Hello, ipmitool maintainers,

I would like to submit several patches which adds some new
functionality
into ipmitool, as well as fix some bugs.

1. [bugs:#305] deferred-activation-fix.diff
This patch fixes the ipmitool HPM.1 agent which mis-recognizes
the
deferred activation support and reports invalid deferred firmware image
version.

2. [bugs:#306] fru-info-fix.diff
This patch removes duplicate output of FRU info #0 when command
fru
print all is sent.
3. [bugs:#307] i82751spt-fix.diff
This patch adds missing check in the LAN+ implementation for
Intel
i82751 MAC which has known deviations from the IPMI v2.0 specification.

4. [patches:#94] vita-support.diff
This patch adds VITA 46.11 specification support to ipmitool.

5. [patches:#95] intf-reopen-fix.diff
This patch provides a solution how to overcome the architectural
ipmitool
drawback which
makes impossible to normally (without hacks) close and re-open
interface.

Please, review.

Regards,
Dmitry

31.03.2014 22:28, Dmitry Bazhenov пишет:

Zdenek,

Here is the updated patch.

Regards,
Dmitry

31.03.2014 13:37, Dmitry Bazhenov пишет:

Zdenek,

Okay then. I'll provide the updated patch later today.

Regards,
Dmitry

31.03.2014 13:34, Zdenek Styblik пишет:

On Mon, Mar 31, 2014 at 9:14 AM, Dmitry Bazhenov <a class="moz-txt-link-rfc2396E" href="mailto:***@pigeonpoint.com"><***@pigeonpoint.com></a>
wrote:

Hello, Zdenek,

I think there should be no such checks inside these callbacks.
However, I guess there should be a check inside thr
ipmi_intf_set_max_request/response_data_size
functions which guarantee that the minimum value will be not less than
25
bytes (required by IPMI spec).

Could you please add such check or is it better for me to provide a new
patch revision?

Regards,
Dmitry

Dmitry,

I don't have access to any IPMI capable hardware, so I'm afraid it's
either up to you or somebody else. I'm sorry.

Best regards,
Z.

31.03.2014 13:07, Zdenek Styblik пишет:

On Thu, Mar 20, 2014 at 6:54 AM, Zdenek Styblik <a class="moz-txt-link-rfc2396E" href="mailto:***@gmail.com"><***@gmail.com></a> wrote:

On Wed, Mar 19, 2014 at 8:33 AM, Dmitry Bazhenov <a class="moz-txt-link-rfc2396E" href="mailto:***@pigeonpoint.com"><***@pigeonpoint.com></a>
wrote:
[...]

I got a bit "scared" by solution applied to
ipmi_intf_get_max_request_data_size() and
ipmi_intf_get_max_response_data_size(). But then I've tried to compile
just this one function with all kinds of switches and compiler didn't
comply, so I guess it's ok.
I wonder, shouldn't be the same logic applied to
ipmi_lanp_set_max_rq_data_size() and ipmi_lanp_set_max_rp_data_size()
as well?

[DB] Calculations in the ipmi_intf_get_max_request_data_size() are
required
for the case if the target IPMC device is accessed via IPMI bridging.
Since
we can not deduce the target channel maximum message size, we use the
minimum required size. These calculations are not needed for direct
IPMC
device access.
[DB] Set max size functions are required if maximum message size over
the
chosen interface must be somehow modified from the value received from
the
interface properties. This is the case for the encrypted RMCP+ payload
where
maximum message size must be reduced by the confidentiality
header/trailer
sizes. Other interface types do not even implement these callbacks.

What I meant is whether under/over-flow shouldn't be checked in those
functions as well.

Ping?

Z.

------------------------------------------------------------------------------
"Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
Instantly run your Selenium tests across 300+ browser/OS combos.
Get unparalleled scalability from the best Selenium testing platform
available
Simple to use. Nothing to install. Get started now for free."
<a class="moz-txt-link-freetext" href="http://p.sf.net/sfu/SauceLabs">http://p.sf.net/sfu/SauceLabs</a>
_______________________________________________
Ipmitool-devel mailing list
<a class="moz-txt-link-abbreviated" href="mailto:Ipmitool-***@lists.sourceforge.net">Ipmitool-***@lists.sourceforge.net</a>
<a class="moz-txt-link-freetext" href="https://lists.sourceforge.net/lists/listinfo/ipmitool-devel">https://lists.sourceforge.net/lists/listinfo/ipmitool-devel</a>

</pre>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
<pre wrap="">
</pre>
</blockquote>
</blockquote>
 
</body>
</html>

Hank Bruning

2014-07-30 19:36:47 UTC

Mark Ov,
is PPS going to answered this ?
hank bruning
Jblade

Post by Dmitry Bazhenov
Hello, Zdenek,
Please, see below.

Post by Dmitry Bazhenov
Hello, Zdenek,

Hello Dmitry,

Post by Dmitry Bazhenov
Could you or someone else from the development team comment

if I had time to do so, I'd have done it already. And I have no ETA
when I'll have time to get back to IPMI tool again, at least not now.

Understood.

Post by Dmitry Bazhenov
There have been no comments on them for two months already.

It's all understood. We did our ipmitool bug-fixes on a paid basis too,
so I understand
your position. I'll pass the response to my employers in order to get
the plan how to
move patches further.

Post by Dmitry Bazhenov
Will appreciate any feedback.

One feedback, though. Fix code formatting, if nothing else. I'm not
going to do it for you, or anybody else, anymore. It got tiresome
after 1+ year of doing so; I can do better things than that; and I
don't have that much of free time to do so(see previous reason).
Sorry for bad news, but reality check was necessary from my point of

view.
Nevermind. Thanks for the support anyway, we appreciate it.
On more thing, can you tell if anyone else beside you administrates
(actively) the project?
Regards,
Dmitry

Post by Zdenek Styblik
Best regards,
Z.

Post by Dmitry Bazhenov
Regards,
Dmitry

On Fri, May 16, 2014 at 2:55 PM, Dmitry Bazhenov <

Post by Dmitry Bazhenov
Hello, all,
Can I expect any progress on the posted patches?
With regards,
Dmitry

Sure,
send over some beers ;)
Z.
--
Zdenek Styblik

Post by Dmitry Bazhenov
Hello, ipmitool maintainers,
I would like to submit several patches which adds some new

functionality

Post by Dmitry Bazhenov
into ipmitool, as well as fix some bugs.
1. [bugs:#305] deferred-activation-fix.diff
This patch fixes the ipmitool HPM.1 agent which mis-recognizes

the

Post by Dmitry Bazhenov
deferred activation support and reports invalid deferred firmware

image

Post by Dmitry Bazhenov
version.
2. [bugs:#306] fru-info-fix.diff
This patch removes duplicate output of FRU info #0 when command

fru

Post by Dmitry Bazhenov
print all is sent.
3. [bugs:#307] i82751spt-fix.diff
This patch adds missing check in the LAN+ implementation for

Intel

Post by Dmitry Bazhenov
i82751 MAC which has known deviations from the IPMI v2.0

specification.

Post by Dmitry Bazhenov
4. [patches:#94] vita-support.diff
This patch adds VITA 46.11 specification support to ipmitool.
5. [patches:#95] intf-reopen-fix.diff
This patch provides a solution how to overcome the architectural ipmitool
drawback which
makes impossible to normally (without hacks) close and re-open interface.
Please, review.
Regards,
Dmitry
Zdenek,
Here is the updated patch.
Regards,
Dmitry
Zdenek,
Okay then. I'll provide the updated patch later today.
Regards,
Dmitry
On Mon, Mar 31, 2014 at 9:14 AM, Dmitry Bazhenov <
Hello, Zdenek,
I think there should be no such checks inside these callbacks.
However, I guess there should be a check inside thr
ipmi_intf_set_max_request/response_data_size
functions which guarantee that the minimum value will be not less

than 25

Post by Dmitry Bazhenov
bytes (required by IPMI spec).
Could you please add such check or is it better for me to provide a

new

Post by Dmitry Bazhenov
patch revision?
Regards,
Dmitry
Dmitry,
I don't have access to any IPMI capable hardware, so I'm afraid it's
either up to you or somebody else. I'm sorry.
Best regards,
Z.
On Thu, Mar 20, 2014 at 6:54 AM, Zdenek Styblik
On Wed, Mar 19, 2014 at 8:33 AM, Dmitry Bazhenov <
[...]
I got a bit "scared" by solution applied to
ipmi_intf_get_max_request_data_size() and
ipmi_intf_get_max_response_data_size(). But then I've tried to compile
just this one function with all kinds of switches and compiler didn't
comply, so I guess it's ok.
I wonder, shouldn't be the same logic applied to
ipmi_lanp_set_max_rq_data_size() and ipmi_lanp_set_max_rp_data_size()
as well?
[DB] Calculations in the ipmi_intf_get_max_request_data_size() are required
for the case if the target IPMC device is accessed via IPMI bridging. Since
we can not deduce the target channel maximum message size, we use the
minimum required size. These calculations are not needed for direct

IPMC

Post by Dmitry Bazhenov
device access.
[DB] Set max size functions are required if maximum message size over the
chosen interface must be somehow modified from the value received from the
interface properties. This is the case for the encrypted RMCP+ payload where
maximum message size must be reduced by the confidentiality header/trailer
sizes. Other interface types do not even implement these callbacks.
What I meant is whether under/over-flow shouldn't be checked in those
functions as well.
Ping?
Z.

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

Post by Dmitry Bazhenov
"Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
Instantly run your Selenium tests across 300+ browser/OS combos.
Get unparalleled scalability from the best Selenium testing platform
available
Simple to use. Nothing to install. Get started now for free."
http://p.sf.net/sfu/SauceLabs
_______________________________________________
Ipmitool-devel mailing list
https://lists.sourceforge.net/lists/listinfo/ipmitool-devel

------------------------------------------------------------------------------
Infragistics Professional
Build stunning WinForms apps today!
Reboot your WinForms applications with our WinForms controls.
Build a bridge from your legacy apps to the future.
http://pubads.g.doubleclick.net/gampad/clk?id=153845071&iu=/4140/ostg.clktrk
_______________________________________________
Ipmitool-devel mailing list
https://lists.sourceforge.net/lists/listinfo/ipmitool-devel

Jim Mankovich

2014-08-08 14:26:16 UTC

Dmitry,

The release of 1.8.15 is not going to come about anytime soon given that
no one has had the time to code review or commit these or any other
bugfix/feature patches recently. Z has been the most active ipmitool
developer for quite some time, and I have been trying to help out when I
can. There really isn't anyone else helping out with ipmitool at this
point in time. I'm the person who has been responsible for cutting the
release as well as doing part time development and code reviews when I
get a chance. I've got the time to continue in this role, and I'll
see if I can free up some more time to get some of the defect fixes into
ipmitool end of Aug or early Sept.

Jim

Post by Dmitry Bazhenov
Hello, Zdenek,
Could you or someone else from the development team comment
#307 Intel I82751 super pass through mode fixup
#319 Interface safe re-open.
#320 VITA 46.11 support
#328 HPM.2 fixes
#329 hpm.1 upgrade fixes
There have been no comments on them for two months already.
We would want them incorporated into the mainline sources before the
1.8.15 release if possible.
Will appreciate any feedback.
Regards,
Dmitry

Post by Dmitry Bazhenov
Hello, all,
Can I expect any progress on the posted patches?
With regards,
Dmitry

Sure,
send over some beers ;)
Z.
--
Zdenek Styblik

------------------------------------------------------------------------------
Want fast and easy access to all the code in your enterprise? Index and
search up to 200,000 lines of code with a free copy of Black Duck
Code Sight - the same software that powers the world's largest code
search on Ohloh, the Black Duck Open Hub! Try it now.
http://p.sf.net/sfu/bds
_______________________________________________
Ipmitool-devel mailing list
https://lists.sourceforge.net/lists/listinfo/ipmitool-devel
--

sandeep khare

2014-08-08 16:24:01 UTC

unknown

1970-01-01 00:00:00 UTC

--001a11c2a3ec19c9be050020a262
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

Someone can please remove me from this group. Thanks

Post by Zdenek Styblik
Dmitry,
The release of 1.8.15 is not going to come about anytime soon given that
no one has had the time to code review or commit these or any other
bugfix/feature patches recently. Z has been the most active ipmitool
developer for quite some time, and I have been trying to help out when I
can. There really isn't anyone else helping out with ipmitool at this
point in time. I'm the person who has been responsible for cutting the
release as well as doing part time development and code reviews when I
get a chance. I've got the time to continue in this role, and I'll
see if I can free up some more time to get some of the defect fixes into
ipmitool end of Aug or early Sept.
Jim

On Fri, May 16, 2014 at 2:55 PM, Dmitry Bazhenov <

Post by Dmitry Bazhenov
Hello, all,
Can I expect any progress on the posted patches?
With regards,
Dmitry

Sure,
send over some beers ;)
Z.
--
Zdenek Styblik

Post by Dmitry Bazhenov
Hello, ipmitool maintainers,
I would like to submit several patches which adds some new

functionality

Post by Dmitry Bazhenov
into ipmitool, as well as fix some bugs.
1. [bugs:#305] deferred-activation-fix.diff
This patch fixes the ipmitool HPM.1 agent which mis-recognizes

the

Post by Dmitry Bazhenov
deferred activation support and reports invalid deferred firmware image
version.
2. [bugs:#306] fru-info-fix.diff
This patch removes duplicate output of FRU info #0 when command

fru

Post by Dmitry Bazhenov
print all is sent.
3. [bugs:#307] i82751spt-fix.diff
This patch adds missing check in the LAN+ implementation for

Intel

Post by Dmitry Bazhenov
i82751 MAC which has known deviations from the IPMI v2.0 specification.
4. [patches:#94] vita-support.diff
This patch adds VITA 46.11 specification support to ipmitool.
5. [patches:#95] intf-reopen-fix.diff
This patch provides a solution how to overcome the architectural

ipmitool

Post by Dmitry Bazhenov
drawback which
makes impossible to normally (without hacks) close and re-open

interface.

Post by Dmitry Bazhenov
Please, review.
Regards,
Dmitry
Zdenek,
Here is the updated patch.
Regards,
Dmitry
Zdenek,
Okay then. I'll provide the updated patch later today.
Regards,
Dmitry
On Mon, Mar 31, 2014 at 9:14 AM, Dmitry Bazhenov <
Hello, Zdenek,
I think there should be no such checks inside these callbacks.
However, I guess there should be a check inside thr
ipmi_intf_set_max_request/response_data_size
functions which guarantee that the minimum value will be not less than

Post by Dmitry Bazhenov
bytes (required by IPMI spec).
Could you please add such check or is it better for me to provide a new
patch revision?
Regards,
Dmitry
Dmitry,
I don't have access to any IPMI capable hardware, so I'm afraid it's
either up to you or somebody else. I'm sorry.
Best regards,
Z.
On Thu, Mar 20, 2014 at 6:54 AM, Zdenek Styblik
On Wed, Mar 19, 2014 at 8:33 AM, Dmitry Bazhenov <
[...]
I got a bit "scared" by solution applied to
ipmi_intf_get_max_request_data_size() and
ipmi_intf_get_max_response_data_size(). But then I've tried to compile
just this one function with all kinds of switches and compiler didn't
comply, so I guess it's ok.
I wonder, shouldn't be the same logic applied to
ipmi_lanp_set_max_rq_data_size() and ipmi_lanp_set_max_rp_data_size()
as well?
[DB] Calculations in the ipmi_intf_get_max_request_data_size() are
required
for the case if the target IPMC device is accessed via IPMI bridging.
Since
we can not deduce the target channel maximum message size, we use the
minimum required size. These calculations are not needed for direct

IPMC

Post by Dmitry Bazhenov
device access.
[DB] Set max size functions are required if maximum message size over
the
chosen interface must be somehow modified from the value received from
the
interface properties. This is the case for the encrypted RMCP+ payload
where
maximum message size must be reduced by the confidentiality
header/trailer
sizes. Other interface types do not even implement these callbacks.
What I meant is whether under/over-flow shouldn't be checked in those
functions as well.
Ping?
Z.

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