All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH phosphor-host-ipmid] Add Set MAC address
@ 2016-02-03 23:40 OpenBMC Patches
  2016-02-03 23:40 ` OpenBMC Patches
  0 siblings, 1 reply; 5+ messages in thread
From: OpenBMC Patches @ 2016-02-03 23:40 UTC (permalink / raw)
  To: openbmc

Add Set MAC address parameter command

https://github.com/openbmc/phosphor-host-ipmid/pull/60

Adriana Kobylak (1):
  Add Set MAC address

 transporthandler.C | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

-- 
2.6.4

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH phosphor-host-ipmid] Add Set MAC address
  2016-02-03 23:40 [PATCH phosphor-host-ipmid] Add Set MAC address OpenBMC Patches
@ 2016-02-03 23:40 ` OpenBMC Patches
  2016-02-04  0:21   ` Joel Stanley
  2016-02-04  5:43   ` Daniel Axtens
  0 siblings, 2 replies; 5+ messages in thread
From: OpenBMC Patches @ 2016-02-03 23:40 UTC (permalink / raw)
  To: openbmc

From: Adriana Kobylak <anoo@us.ibm.com>

Add Set MAC address parameter command
---
 transporthandler.C | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/transporthandler.C b/transporthandler.C
index dac8fe8..c38adea 100644
--- a/transporthandler.C
+++ b/transporthandler.C
@@ -14,6 +14,7 @@
 #endif
 
 // OpenBMC System Manager dbus framework
+extern sd_bus *bus;
 const char  *app   =  "org.openbmc.NetworkManager";
 const char  *obj   =  "/org/openbmc/NetworkManager/Interface";
 const char  *ifc   =  "org.openbmc.NetworkManager";
@@ -64,6 +65,37 @@ ipmi_ret_t ipmi_transport_set_lan(ipmi_netfn_t netfn, ipmi_cmd_t cmd,
     {
         sprintf(new_ipaddr, "%d.%d.%d.%d", reqptr->data[0], reqptr->data[1], reqptr->data[2], reqptr->data[3]);
     }
+    else if (reqptr->parameter == 5) // MAC
+    {
+        char                mac[18];
+        sd_bus_message *reply = NULL, *m = NULL;
+        sd_bus_error error = SD_BUS_ERROR_NULL;
+        int r = 0;
+
+        sprintf(mac, "%x:%x:%x:%x:%x:%x",
+                reqptr->data[0],
+                reqptr->data[1],
+                reqptr->data[2],
+                reqptr->data[3],
+                reqptr->data[4],
+                reqptr->data[5]);
+
+        r = sd_bus_message_new_method_call(bus,&m,app,obj,ifc,"SetHwAddress");
+        if (r < 0) {
+            fprintf(stderr, "Failed to add method object: %s\n", strerror(-r));
+            return -1;
+        }
+        r = sd_bus_message_append(m, "s", mac);
+        if (r < 0) {
+            fprintf(stderr, "Failed to append message data: %s\n", strerror(-r));
+            return -1;
+        }
+        r = sd_bus_call(bus, m, 0, &error, &reply);
+        if (r < 0) {
+            fprintf(stderr, "Failed to call method: %s\n", strerror(-r));
+            return -1;
+        }
+    }
     else if (reqptr->parameter == 6) // Subnet
     {
         sprintf(new_netmask, "%d.%d.%d.%d", reqptr->data[0], reqptr->data[1], reqptr->data[2], reqptr->data[3]);
-- 
2.6.4

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH phosphor-host-ipmid] Add Set MAC address
  2016-02-03 23:40 ` OpenBMC Patches
@ 2016-02-04  0:21   ` Joel Stanley
  2016-02-04  0:51     ` Norman James
  2016-02-04  5:43   ` Daniel Axtens
  1 sibling, 1 reply; 5+ messages in thread
From: Joel Stanley @ 2016-02-04  0:21 UTC (permalink / raw)
  To: OpenBMC Patches; +Cc: OpenBMC Maillist

On Thu, Feb 4, 2016 at 10:40 AM, OpenBMC Patches
<openbmc-patches@stwcx.xyz> wrote:
> From: Adriana Kobylak <anoo@us.ibm.com>
>
> Add Set MAC address parameter command

I notice you posted this patch 40 minutes ago, but had already merged
it. That doesn't give much opportunity for review.

> ---
>  transporthandler.C | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>
> diff --git a/transporthandler.C b/transporthandler.C
> index dac8fe8..c38adea 100644
> --- a/transporthandler.C
> +++ b/transporthandler.C
> @@ -14,6 +14,7 @@
>  #endif
>
>  // OpenBMC System Manager dbus framework
> +extern sd_bus *bus;

Don't use extern.

We have ipmid_get_sd_bus_connection() to get a pointer to the sd_bus.

>  const char  *app   =  "org.openbmc.NetworkManager";
>  const char  *obj   =  "/org/openbmc/NetworkManager/Interface";
>  const char  *ifc   =  "org.openbmc.NetworkManager";
> @@ -64,6 +65,37 @@ ipmi_ret_t ipmi_transport_set_lan(ipmi_netfn_t netfn, ipmi_cmd_t cmd,
>      {
>          sprintf(new_ipaddr, "%d.%d.%d.%d", reqptr->data[0], reqptr->data[1], reqptr->data[2], reqptr->data[3]);
>      }
> +    else if (reqptr->parameter == 5) // MAC
> +    {
> +        char                mac[18];
> +        sd_bus_message *reply = NULL, *m = NULL;
> +        sd_bus_error error = SD_BUS_ERROR_NULL;
> +        int r = 0;

I don't think any of these should be initialised here.

> +
> +        sprintf(mac, "%x:%x:%x:%x:%x:%x",

You want to use snprintf here.

Also, does this work? I would have thought you'd need to do %02x to
ensure leading zeroes are in the string.

> +                reqptr->data[0],
> +                reqptr->data[1],
> +                reqptr->data[2],
> +                reqptr->data[3],
> +                reqptr->data[4],
> +                reqptr->data[5]);
> +
> +        r = sd_bus_message_new_method_call(bus,&m,app,obj,ifc,"SetHwAddress");
> +        if (r < 0) {
> +            fprintf(stderr, "Failed to add method object: %s\n", strerror(-r));
> +            return -1;
> +        }
> +        r = sd_bus_message_append(m, "s", mac);
> +        if (r < 0) {
> +            fprintf(stderr, "Failed to append message data: %s\n", strerror(-r));
> +            return -1;
> +        }
> +        r = sd_bus_call(bus, m, 0, &error, &reply);
> +        if (r < 0) {
> +            fprintf(stderr, "Failed to call method: %s\n", strerror(-r));
> +            return -1;
> +        }
> +    }
>      else if (reqptr->parameter == 6) // Subnet
>      {
>          sprintf(new_netmask, "%d.%d.%d.%d", reqptr->data[0], reqptr->data[1], reqptr->data[2], reqptr->data[3]);
> --
> 2.6.4
>
>
> _______________________________________________
> openbmc mailing list
> openbmc@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/openbmc

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH phosphor-host-ipmid] Add Set MAC address
  2016-02-04  0:21   ` Joel Stanley
@ 2016-02-04  0:51     ` Norman James
  0 siblings, 0 replies; 5+ messages in thread
From: Norman James @ 2016-02-04  0:51 UTC (permalink / raw)
  To: Joel Stanley; +Cc: OpenBMC Patches, OpenBMC Maillist


[-- Attachment #1.1: Type: text/plain, Size: 3604 bytes --]


We are late on tagging a release for Rackspace and so pushing through some
final fixes.  We can open an issue to remind us to review later.


Regards,
Norman James
IBM - POWER Systems Architect
Phone: 1-512-286-6807 (T/L: 363-6807)
Internet: njames@us.ibm.com




From:	Joel Stanley <joel@jms.id.au>
To:	OpenBMC Patches <openbmc-patches@stwcx.xyz>
Cc:	OpenBMC Maillist <openbmc@lists.ozlabs.org>
Date:	02/03/2016 06:22 PM
Subject:	Re: [PATCH phosphor-host-ipmid] Add Set MAC address
Sent by:	"openbmc" <openbmc-bounces+njames=us.ibm.com@lists.ozlabs.org>



On Thu, Feb 4, 2016 at 10:40 AM, OpenBMC Patches
<openbmc-patches@stwcx.xyz> wrote:
> From: Adriana Kobylak <anoo@us.ibm.com>
>
> Add Set MAC address parameter command

I notice you posted this patch 40 minutes ago, but had already merged
it. That doesn't give much opportunity for review.

> ---
>  transporthandler.C | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>
> diff --git a/transporthandler.C b/transporthandler.C
> index dac8fe8..c38adea 100644
> --- a/transporthandler.C
> +++ b/transporthandler.C
> @@ -14,6 +14,7 @@
>  #endif
>
>  // OpenBMC System Manager dbus framework
> +extern sd_bus *bus;

Don't use extern.

We have ipmid_get_sd_bus_connection() to get a pointer to the sd_bus.

>  const char  *app   =  "org.openbmc.NetworkManager";
>  const char  *obj   =  "/org/openbmc/NetworkManager/Interface";
>  const char  *ifc   =  "org.openbmc.NetworkManager";
> @@ -64,6 +65,37 @@ ipmi_ret_t ipmi_transport_set_lan(ipmi_netfn_t netfn,
ipmi_cmd_t cmd,
>      {
>          sprintf(new_ipaddr, "%d.%d.%d.%d", reqptr->data[0], reqptr->data
[1], reqptr->data[2], reqptr->data[3]);
>      }
> +    else if (reqptr->parameter == 5) // MAC
> +    {
> +        char                mac[18];
> +        sd_bus_message *reply = NULL, *m = NULL;
> +        sd_bus_error error = SD_BUS_ERROR_NULL;
> +        int r = 0;

I don't think any of these should be initialised here.

> +
> +        sprintf(mac, "%x:%x:%x:%x:%x:%x",

You want to use snprintf here.

Also, does this work? I would have thought you'd need to do %02x to
ensure leading zeroes are in the string.

> +                reqptr->data[0],
> +                reqptr->data[1],
> +                reqptr->data[2],
> +                reqptr->data[3],
> +                reqptr->data[4],
> +                reqptr->data[5]);
> +
> +        r = sd_bus_message_new_method_call
(bus,&m,app,obj,ifc,"SetHwAddress");
> +        if (r < 0) {
> +            fprintf(stderr, "Failed to add method object: %s\n",
strerror(-r));
> +            return -1;
> +        }
> +        r = sd_bus_message_append(m, "s", mac);
> +        if (r < 0) {
> +            fprintf(stderr, "Failed to append message data: %s\n",
strerror(-r));
> +            return -1;
> +        }
> +        r = sd_bus_call(bus, m, 0, &error, &reply);
> +        if (r < 0) {
> +            fprintf(stderr, "Failed to call method: %s\n", strerror
(-r));
> +            return -1;
> +        }
> +    }
>      else if (reqptr->parameter == 6) // Subnet
>      {
>          sprintf(new_netmask, "%d.%d.%d.%d", reqptr->data[0], reqptr->
data[1], reqptr->data[2], reqptr->data[3]);
> --
> 2.6.4
>
>
> _______________________________________________
> openbmc mailing list
> openbmc@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/openbmc
_______________________________________________
openbmc mailing list
openbmc@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/openbmc


[-- Attachment #1.2: Type: text/html, Size: 6249 bytes --]

[-- Attachment #2: graycol.gif --]
[-- Type: image/gif, Size: 105 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH phosphor-host-ipmid] Add Set MAC address
  2016-02-03 23:40 ` OpenBMC Patches
  2016-02-04  0:21   ` Joel Stanley
@ 2016-02-04  5:43   ` Daniel Axtens
  1 sibling, 0 replies; 5+ messages in thread
From: Daniel Axtens @ 2016-02-04  5:43 UTC (permalink / raw)
  To: OpenBMC Patches, openbmc

[-- Attachment #1: Type: text/plain, Size: 1730 bytes --]

Hi,

>      {
>          sprintf(new_ipaddr, "%d.%d.%d.%d", reqptr->data[0], reqptr->data[1], reqptr->data[2], reqptr->data[3]);
>      }
> +    else if (reqptr->parameter == 5) // MAC
Instead of a comment, could we #define or 'const' that 5 == MAC?

Regards,
Daniel
> +    {
> +        char                mac[18];
> +        sd_bus_message *reply = NULL, *m = NULL;
> +        sd_bus_error error = SD_BUS_ERROR_NULL;
> +        int r = 0;
> +
> +        sprintf(mac, "%x:%x:%x:%x:%x:%x",
> +                reqptr->data[0],
> +                reqptr->data[1],
> +                reqptr->data[2],
> +                reqptr->data[3],
> +                reqptr->data[4],
> +                reqptr->data[5]);
> +
> +        r = sd_bus_message_new_method_call(bus,&m,app,obj,ifc,"SetHwAddress");
> +        if (r < 0) {
> +            fprintf(stderr, "Failed to add method object: %s\n", strerror(-r));
> +            return -1;
> +        }
> +        r = sd_bus_message_append(m, "s", mac);
> +        if (r < 0) {
> +            fprintf(stderr, "Failed to append message data: %s\n", strerror(-r));
> +            return -1;
> +        }
> +        r = sd_bus_call(bus, m, 0, &error, &reply);
> +        if (r < 0) {
> +            fprintf(stderr, "Failed to call method: %s\n", strerror(-r));
> +            return -1;
> +        }
> +    }
>      else if (reqptr->parameter == 6) // Subnet
>      {
>          sprintf(new_netmask, "%d.%d.%d.%d", reqptr->data[0], reqptr->data[1], reqptr->data[2], reqptr->data[3]);
> -- 
> 2.6.4
>
>
> _______________________________________________
> openbmc mailing list
> openbmc@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/openbmc

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 859 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2016-02-04  5:43 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-03 23:40 [PATCH phosphor-host-ipmid] Add Set MAC address OpenBMC Patches
2016-02-03 23:40 ` OpenBMC Patches
2016-02-04  0:21   ` Joel Stanley
2016-02-04  0:51     ` Norman James
2016-02-04  5:43   ` Daniel Axtens

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.