[PATCH phosphor-host-ipmid] Add Set MAC address

[Norman James]

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 at us.ibm.com




From:	Joel Stanley <joel at jms.id.au>
To:	OpenBMC Patches <openbmc-patches at stwcx.xyz>
Cc:	OpenBMC Maillist <openbmc at 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 at lists.ozlabs.org>



On Thu, Feb 4, 2016 at 10:40 AM, OpenBMC Patches
<openbmc-patches at stwcx.xyz> wrote:
> From: Adriana Kobylak <anoo at 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 at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/openbmc
_______________________________________________
openbmc mailing list
openbmc at lists.ozlabs.org
https://lists.ozlabs.org/listinfo/openbmc

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20160203/cbbf322c/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: graycol.gif
Type: image/gif
Size: 105 bytes
Desc: not available
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20160203/cbbf322c/attachment.gif>